[Qemu-devel] [Bug 1243287] Re: [KVM/QEMU][ARM][SAUCY] fails to boot cloud-image due to host kvm fail

[Andre Przywara]

Peter, the config option is called: CONFIG_STRICT_DEVMEM
And /dev/mem behaves differently between doing read() and doing mmap().

As Peter already hinted, the memory layout is different on native Midway (which 
has DRAM starting at 0) and mach-virt/vexpress (which start at 128MB / 2GB 
respectively). So accessing 0xf reads memory on native, but faults in stage 
2 mapping in the host kernel.
But the actual problem is that the host kernel passes this abort on to 
userspace, where QEMU just does abort(), thus crashing the whole virtual 
machine. Actually I think we should change the host kernel to inject the data 
abort into the guest. I tested this with a simple patch, it now makes dmidecode 
crash (due to a signal), but keeps the guest alive.

This approach needs some discussion on the ML, I guess.

But near term we should not call dmidecode on ARM, since AFAIK it does
not give any information. There once was a discussion on patching
dmidecode to output hardcoded or elsewhere gathered information to make
scripts happy, but I don't know where this ended.

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

Title:
  [KVM/QEMU][ARM][SAUCY] fails to boot cloud-image due to host kvm fail

Status in QEMU:
  Confirmed

Bug description:
  On booting the cloud image using qemu/kvm fails with the following
  error:

  Cloud-init v. 0.7.3 running 'init' at Thu, 03 Oct 2013 16:45:21 +. Up 
360.78 seconds.
  ci-info: +Net device info+
  ci-info: ++--+---+---+---+
  ci-info: | Device | Up | Address | Mask | Hw-Address |
  ci-info: ++--+---+---+---+
  ci-info: | lo | True | 127.0.0.1 | 255.0.0.0 | . |
  ci-info: | eth0 | True | 10.0.2.15 | 255.255.255.0 | 52:54:00:12:34:56 |
  ci-info: ++--+---+---+---+
  ci-info: ++Route 
info++
  ci-info: 
+---+-+--+---+---+---+
  ci-info: | Route | Destination | Gateway | Genmask | Interface | Flags |
  ci-info: 
+---+-+--+---+---+---+
  ci-info: | 0 | 0.0.0.0 | 10.0.2.2 | 0.0.0.0 | eth0 | UG |
  ci-info: | 1 | 10.0.2.0 | 0.0.0.0 | 255.255.255.0 | eth0 | U |
  ci-info: 
+---+-+--+---+---+---+
  error: kvm run failed Function not implemented

  /usr/lib/python2.7/dist-
  packages/cloudinit/sources/DataSourceAltCloud.py assumes that
  dmidecode command is availabe (ie it assumes that system is x86) on
  arm systems there is no dmidecode command so host kvm fails with the
  message error: kvm run failed Function not implemented

  The patch makes get_cloud_type() function return with UNKNOWN for ARM
  systems.

  I was able to boot the cloud-image on ARM after applying this change.

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

[Qemu-devel] [PATCH v2 0/2] add initial Calxeda Midway A15 support

[Andre Przywara]

While the Calxeda Midway part is actually a bit more than a Highbank
with A15s, for QEMU's purposes this view is sufficient.
So refactor the Highbank initialization to allow a very similar
machine and add the necessary changes to support the new machine
called midway.

This is based on work by Rob Herring rob.herr...@calxeda.com.

Signed-off-by: Andre Przywara andre.przyw...@calxeda.com

Andre Przywara (2):
  ARM/highbank: prepare for adding similar machines
  ARM/highbank: add support for Calxeda ECX-2000 / Midway

 hw/arm/highbank.c | 61 +--
 1 file changed, 50 insertions(+), 11 deletions(-)

-- 
1.7.12.1

[Qemu-devel] [PATCH v2 1/2] ARM/highbank: prepare for adding similar machines

[Andre Przywara]

To allow the modelling of machines similar to Calxeda Highbank,
introduce a parameter to the init function and call it from a
wrapper. This allows to tweak the definition for individual machines
later on.

Signed-off-by: Andre Przywara andre.przyw...@calxeda.com
---
 hw/arm/highbank.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 4405dbd..2c32a2b 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -183,20 +183,24 @@ type_init(highbank_regs_register_types)
 
 static struct arm_boot_info highbank_binfo;
 
+enum cxmachines {
+CALXEDA_HIGHBANK,
+};
+
 /* ram_size must be set to match the upper bound of memory in the
  * device tree (linux/arch/arm/boot/dts/highbank.dts), which is
  * normally 0xff90 or -m 4089. When running this board on a
  * 32-bit host, set the reg value of memory to 0xf7ff0 in the
  * device tree and pass -m 2047 to QEMU.
  */
-static void highbank_init(QEMUMachineInitArgs *args)
+static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine)
 {
 ram_addr_t ram_size = args-ram_size;
 const char *cpu_model = args-cpu_model;
 const char *kernel_filename = args-kernel_filename;
 const char *kernel_cmdline = args-kernel_cmdline;
 const char *initrd_filename = args-initrd_filename;
-DeviceState *dev;
+DeviceState *dev = NULL;
 SysBusDevice *busdev;
 qemu_irq *irqp;
 qemu_irq pic[128];
@@ -208,7 +212,11 @@ static void highbank_init(QEMUMachineInitArgs *args)
 char *sysboot_filename;
 
 if (!cpu_model) {
-cpu_model = cortex-a9;
+switch (machine) {
+case CALXEDA_HIGHBANK:
+cpu_model = cortex-a9;
+break;
+}
 }
 
 for (n = 0; n  smp_cpus; n++) {
@@ -246,7 +254,11 @@ static void highbank_init(QEMUMachineInitArgs *args)
 }
 }
 
-dev = qdev_create(NULL, a9mpcore_priv);
+switch (machine) {
+case CALXEDA_HIGHBANK:
+dev = qdev_create(NULL, a9mpcore_priv);
+break;
+}
 qdev_prop_set_uint32(dev, num-cpu, smp_cpus);
 qdev_prop_set_uint32(dev, num-irq, NIRQ_GIC);
 qdev_init_nofail(dev);
@@ -324,6 +336,11 @@ static void highbank_init(QEMUMachineInitArgs *args)
 arm_load_kernel(arm_env_get_cpu(first_cpu), highbank_binfo);
 }
 
+static void highbank_init(QEMUMachineInitArgs *args)
+{
+calxeda_init(args, CALXEDA_HIGHBANK);
+}
+
 static QEMUMachine highbank_machine = {
 .name = highbank,
 .desc = Calxeda Highbank (ECX-1000),
@@ -333,9 +350,9 @@ static QEMUMachine highbank_machine = {
 DEFAULT_MACHINE_OPTIONS,
 };
 
-static void highbank_machine_init(void)
+static void calxeda_machines_init(void)
 {
 qemu_register_machine(highbank_machine);
 }
 
-machine_init(highbank_machine_init);
+machine_init(calxeda_machines_init);
-- 
1.7.12.1

[Qemu-devel] [PATCH v2 2/2] ARM/highbank: add support for Calxeda ECX-2000 / Midway

[Andre Przywara]

The Calxeda ECX-2000 chip (aka. Midway) is model-wise quite similar
to the Highbank. The most prominent difference is the Cortex-A15 CPU
core in it, together with the associated core peripherals.

Add a new ARM machine type called midway.
Move the L2 cache controller device into the Highbank specific part,
since Midway does not have (and need) it.

Signed-off-by: Andre Przywara andre.przyw...@calxeda.com
---
 hw/arm/highbank.c | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 2c32a2b..3c99a81 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -185,6 +185,7 @@ static struct arm_boot_info highbank_binfo;
 
 enum cxmachines {
 CALXEDA_HIGHBANK,
+CALXEDA_MIDWAY,
 };
 
 /* ram_size must be set to match the upper bound of memory in the
@@ -216,6 +217,9 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 case CALXEDA_HIGHBANK:
 cpu_model = cortex-a9;
 break;
+case CALXEDA_MIDWAY:
+cpu_model = cortex-a15;
+break;
 }
 }
 
@@ -256,8 +260,16 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 
 switch (machine) {
 case CALXEDA_HIGHBANK:
+dev = qdev_create(NULL, l2x0);
+qdev_init_nofail(dev);
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, 0xfff12000);
+
 dev = qdev_create(NULL, a9mpcore_priv);
 break;
+case CALXEDA_MIDWAY:
+dev = qdev_create(NULL, a15mpcore_priv);
+break;
 }
 qdev_prop_set_uint32(dev, num-cpu, smp_cpus);
 qdev_prop_set_uint32(dev, num-irq, NIRQ_GIC);
@@ -272,11 +284,6 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 pic[n] = qdev_get_gpio_in(dev, n);
 }
 
-dev = qdev_create(NULL, l2x0);
-qdev_init_nofail(dev);
-busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, 0xfff12000);
-
 dev = qdev_create(NULL, sp804);
 qdev_prop_set_uint32(dev, freq0, 15000);
 qdev_prop_set_uint32(dev, freq1, 15000);
@@ -341,6 +348,11 @@ static void highbank_init(QEMUMachineInitArgs *args)
 calxeda_init(args, CALXEDA_HIGHBANK);
 }
 
+static void midway_init(QEMUMachineInitArgs *args)
+{
+calxeda_init(args, CALXEDA_MIDWAY);
+}
+
 static QEMUMachine highbank_machine = {
 .name = highbank,
 .desc = Calxeda Highbank (ECX-1000),
@@ -350,9 +362,19 @@ static QEMUMachine highbank_machine = {
 DEFAULT_MACHINE_OPTIONS,
 };
 
+static QEMUMachine midway_machine = {
+.name = midway,
+.desc = Calxeda Midway (ECX-2000),
+.init = midway_init,
+.block_default_type = IF_SCSI,
+.max_cpus = 4,
+DEFAULT_MACHINE_OPTIONS,
+};
+
 static void calxeda_machines_init(void)
 {
 qemu_register_machine(highbank_machine);
+qemu_register_machine(midway_machine);
 }
 
 machine_init(calxeda_machines_init);
-- 
1.7.12.1

[Qemu-devel] [PATCH] highbank: add initial Calxeda Midway A15 support

[Andre Przywara]

From: Rob Herring rob.herr...@calxeda.com

While the Calxeda Midway part is actually a bit more than a Highbank
with A15s, for QEMU's purposes this view is sufficient. So to allow
both emulation with that chip as well as KVM guests using that model
add an A15 CPU and it's peripherals as an option. The use of:
-M highbank -cpu cortex-a15 simply gives the new chip without the
need for a new model.

Signed-off-by: Rob Herring rob.herr...@calxeda.com
Signed-off-by: Andre Przywara andre.przyw...@calxeda.com
---
 hw/arm/highbank.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 4405dbd..ed864c6 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -196,6 +196,7 @@ static void highbank_init(QEMUMachineInitArgs *args)
 const char *kernel_filename = args-kernel_filename;
 const char *kernel_cmdline = args-kernel_cmdline;
 const char *initrd_filename = args-initrd_filename;
+CPUARMState *env = NULL;
 DeviceState *dev;
 SysBusDevice *busdev;
 qemu_irq *irqp;
@@ -223,6 +224,8 @@ static void highbank_init(QEMUMachineInitArgs *args)
 cpu-reset_cbar = GIC_BASE_ADDR;
 irqp = arm_pic_init_cpu(cpu);
 cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
+
+env = cpu-env;
 }
 
 sysmem = get_system_memory();
@@ -246,7 +249,16 @@ static void highbank_init(QEMUMachineInitArgs *args)
 }
 }
 
-dev = qdev_create(NULL, a9mpcore_priv);
+if (arm_feature(env, ARM_FEATURE_LPAE)) {
+dev = qdev_create(NULL, a15mpcore_priv);
+} else {
+dev = qdev_create(NULL, l2x0);
+qdev_init_nofail(dev);
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, 0xfff12000);
+
+dev = qdev_create(NULL, a9mpcore_priv);
+}
 qdev_prop_set_uint32(dev, num-cpu, smp_cpus);
 qdev_prop_set_uint32(dev, num-irq, NIRQ_GIC);
 qdev_init_nofail(dev);
@@ -260,11 +272,6 @@ static void highbank_init(QEMUMachineInitArgs *args)
 pic[n] = qdev_get_gpio_in(dev, n);
 }
 
-dev = qdev_create(NULL, l2x0);
-qdev_init_nofail(dev);
-busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, 0xfff12000);
-
 dev = qdev_create(NULL, sp804);
 qdev_prop_set_uint32(dev, freq0, 15000);
 qdev_prop_set_uint32(dev, freq1, 15000);
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH arm-devs v2 8/8] arm/highbank.c: Fix MPCore periphbase name

[Andre Przywara]

On 11/28/2013 08:41 PM, Peter Maydell wrote:

(CCing Rob)


On 28 November 2013 03:31, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:

GIC_BASE_ADDR is not the base address of the GIC. Its clear from the
code that this is the base address of the MPCore. Rename to
MPCORE_PERIPHBASE accordingly.


MPCore is one of those terms I dislike because it
doesn't actually match up with the documentation's
terminology...


Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

  hw/arm/highbank.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index cb32325..be2cc20 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -28,11 +28,11 @@
  #include exec/address-spaces.h
  #include qemu/error-report.h

-#define SMP_BOOT_ADDR 0x100
-#define SMP_BOOT_REG  0x40
-#define GIC_BASE_ADDR 0xfff1
+#define SMP_BOOT_ADDR   0x100
+#define SMP_BOOT_REG0x40
+#define MPCORE_PERIPHBASE   0xfff1

-#define NIRQ_GIC  160
+#define NIRQ_GIC160

  /* Board init.  */

@@ -55,7 +55,7 @@ static void hb_write_secondary(ARMCPU *cpu, const struct 
arm_boot_info *info)
  0xe1110001, /* tst r1, r1 */
  0x0afb, /* beq wfi */
  0xe12fff11, /* bx  r1 */
-GIC_BASE_ADDR  /* privbase: gic address.  */
+MPCORE_PERIPHBASE   /* privbase: gic address.  */


Comment no longer matches code.

More seriously, this secondary-boot code has a bunch
of hardcoded offsets for GIC registers which are
correct for the A9 but wrong for A15. Andre, did you
test Midway with an SMP boot config?


Obviously not ;-)
In the moment I cannot care about this, but it looks like we need 
different SMP stubs for Highbank and Midway.

Thanks for the hint,

Andre.



The generic hw/arm/boot code dealt with this problem
by feeding the boot code the GIC CPU interface base
address rather than the generic peripheral base address;
the offsets within the CPU i/f are the same for both
cores.


  };
  for (n = 0; n  ARRAY_SIZE(smpboot); n++) {
  smpboot[n] = tswap32(smpboot[n]);
@@ -236,7 +236,8 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)

  cpu = ARM_CPU(object_new(object_class_get_name(oc)));

-object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, reset-cbar, 
err);
+object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, reset-cbar,
+err);
  if (err) {
  error_report(%s, error_get_pretty(err));
  exit(1);
@@ -287,7 +288,7 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
  qdev_prop_set_uint32(dev, num-irq, NIRQ_GIC);
  qdev_init_nofail(dev);
  busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, GIC_BASE_ADDR);
+sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
  for (n = 0; n  smp_cpus; n++) {
  sysbus_connect_irq(busdev, n, cpu_irq[n]);
  }
--
1.8.4.4



thanks
-- PMM

[Qemu-devel] [PATCH] kvm/x86: enlarge number of possible CPUID leaves

[Andre Przywara]

Currently the number of CPUID leaves KVM handles is limited to 40.
My desktop machine (AthlonII) already has 35 and future CPUs will
expand this well beyond the limit. Extend the limit to 80 to make
room for future processors.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 arch/x86/include/asm/kvm_host.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Hi,
I found that either KVM or QEMU (possibly both) are broken in respect
to handling more CPUID entries than the limit dictates. KVM will
return -E2BIG, which is the same error as if the user hasn't provided
enough space to hold all entries. Now QEMU will continue to enlarge
the allocated memory until it gets into an out-of-memory condition.
I have tried to fix this with teaching KVM how to deal with a capped
number of entries (there are some bugs in the current code), but this
will limit the number of CPUID entries KVM handles, which will surely
cut of the lastly appended PV leaves.
A proper fix would be to make this allocation dynamic. Is this a
feasible way or will this lead to issues or side-effects?

Regards,
Andre.

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 54e42c8..3cc80c4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -79,7 +79,7 @@
 #define KVM_NUM_MMU_PAGES (1  KVM_MMU_HASH_SHIFT)
 #define KVM_MIN_FREE_MMU_PAGES 5
 #define KVM_REFILL_PAGES 25
-#define KVM_MAX_CPUID_ENTRIES 40
+#define KVM_MAX_CPUID_ENTRIES 80
 #define KVM_NR_FIXED_MTRR_REGION 88
 #define KVM_NR_VAR_MTRR 8
 
-- 
1.6.4

[Qemu-devel] Re: [PATCH] kvm/x86: enlarge number of possible CPUID leaves

[Andre Przywara]

Avi Kivity wrote:

On 12/01/2010 01:17 PM, Andre Przywara wrote:

Currently the number of CPUID leaves KVM handles is limited to 40.
My desktop machine (AthlonII) already has 35 and future CPUs will
expand this well beyond the limit. Extend the limit to 80 to make
room for future processors.

Signed-off-by: Andre Przywaraandre.przyw...@amd.com
---
  arch/x86/include/asm/kvm_host.h |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

Hi,
I found that either KVM or QEMU (possibly both) are broken in respect
to handling more CPUID entries than the limit dictates. KVM will
return -E2BIG, which is the same error as if the user hasn't provided
enough space to hold all entries. Now QEMU will continue to enlarge
the allocated memory until it gets into an out-of-memory condition.
I have tried to fix this with teaching KVM how to deal with a capped
number of entries (there are some bugs in the current code), but this
will limit the number of CPUID entries KVM handles, which will surely
cut of the lastly appended PV leaves.
A proper fix would be to make this allocation dynamic. Is this a
feasible way or will this lead to issues or side-effects?



Well, there has to be some limit, or userspace can allocate unbounded 
kernel memory.
But this limit should not be a compile-time constant, but a runtime one. 
The needed size depends on the host CPU (plus the KVM PV leaves) and 
thus could be determined once for all VMs and vCPUs at module load-time. 
But then we cannot use the static array allocation we currently have in 
struct kvm_vcpu_arch:

struct kvm_cpuid_entry2 cpuid_entries[KVM_MAX_CPUID_ENTRIES];
So we would use a kind-of dynamic allocation bounded by the host CPU's 
need. But for the code is does not make much difference to a real 
dynamic allocation.


Also we could implement kvm_dev_ioctl_get_supported_cpuid without the 
vmalloc, if we don't care about some dozens of copy_to_user() calls in 
this function. Then we would not need this limit in GET_SUPPORTED_CPUID 
at all, but it will strike us again at KVM_SET_CPUID[2], where we may 
not fulfill the promises we gave earlier.

Having said this, what about that:
kvm_dev_ioctl_get_supported_cpuid is invariant to the VM or vCPU (as it 
is used by a system ioctl), so it could be run once at initialization, 
which would limit the ioctl implementation to a plain bounded copy.

Would you want such a patch (removing the vmalloc and maybe even the limit)?

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

[Qemu-devel] Re: [PATCH] kvm/x86: enlarge number of possible CPUID leaves

[Andre Przywara]

Avi, Marcello,

can you please commit this simple fix? (turning 40 to 80?)
Without it QEMU crashes reliably on our new CPUs (they return 46 leaves) 
and causes pain in our testing, because we have to manually apply this 
patch on each tree.


Thanks!
Andre.


Currently the number of CPUID leaves KVM handles is limited to 40.
My desktop machine (AthlonII) already has 35 and future CPUs will
expand this well beyond the limit. Extend the limit to 80 to make
room for future processors.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 arch/x86/include/asm/kvm_host.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Hi,
I found that either KVM or QEMU (possibly both) are broken in respect
to handling more CPUID entries than the limit dictates. KVM will
return -E2BIG, which is the same error as if the user hasn't provided
enough space to hold all entries. Now QEMU will continue to enlarge
the allocated memory until it gets into an out-of-memory condition.
I have tried to fix this with teaching KVM how to deal with a capped
number of entries (there are some bugs in the current code), but this
will limit the number of CPUID entries KVM handles, which will surely
cut of the lastly appended PV leaves.
A proper fix would be to make this allocation dynamic. Is this a
feasible way or will this lead to issues or side-effects?

Regards,
Andre.

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 54e42c8..3cc80c4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -79,7 +79,7 @@
 #define KVM_NUM_MMU_PAGES (1  KVM_MMU_HASH_SHIFT)
 #define KVM_MIN_FREE_MMU_PAGES 5
 #define KVM_REFILL_PAGES 25
-#define KVM_MAX_CPUID_ENTRIES 40
+#define KVM_MAX_CPUID_ENTRIES 80
 #define KVM_NR_FIXED_MTRR_REGION 88
 #define KVM_NR_VAR_MTRR 8
 

Re: [Qemu-devel] [RFC] Bug Day - June 1st, 2010

[Andre Przywara]

Michael Tokarev wrote:

...

Also, thanks to Andre Przywara, whole winNT thing works but it requires
-cpu qemu64,level=1 (or level=2 or =3), -- _not_ with default CPU.  This
is also testing, but it's not obvious what to do witht the result...
Can't we use the file based CPU models for that? Actually it looks like 
a template config file for certain guest operation systems (like -vga 
std -net nic,model=ne2k_pci for older Windows version) make sense. These 
could include all quirks that we find on the way.


BTW: Does anyone knows what the problem with Windows95/98 on KVM is? I 
tried some tracing today, but couldn't find a hint.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

[Qemu-devel] [Bug 267542] Re: MINIX 3 won't boot in qemu 0.9.1

[Andre Przywara]

Is that still a problem? What was the exact error?
I quickly tried the 3.1.2a on qemu 0.12.4 (with and without KVM) and I could 
easily login.


** Changed in: qemu
   Status: New = Incomplete

-- 
MINIX 3 won't boot in qemu 0.9.1
https://bugs.launchpad.net/bugs/267542
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: Incomplete

Bug description:
CD Image 3.1.2a was downloaded from http://www.minix3.org/download/

It booted with previous version of qemu but hangs at startup with 0.9.1.

Hardware acceleration is disabled.

Please ask if there is other information I can give you.

Re: [Qemu-devel] [RFC] Bug Day - June 1st, 2010

[Andre Przywara]

Michael Tokarev wrote:

20.05.2010 02:30, Anthony Liguori wrote:

On 05/19/2010 05:29 PM, Andre Przywara wrote:

Michael Tokarev wrote:

...

Also, thanks to Andre Przywara, whole winNT thing works but it requires
-cpu qemu64,level=1 (or level=2 or =3), -- _not_ with default CPU. This

[]

It'd be nice if we had more flexibility in defining custom machine types
so you could just do qemu -M win98.


This is wrong IMHO.  win98 and winNT can run on various different
machines, including all modern ones (yes I tried the same winNT
on my Athlon X2-64, just had to switch SATA from AHCI to IDE;
win95 works too)...  just not in kvm :)
Well, not really. You were lucky with your Athlon X2-64, actually it is 
the last machine not triggering the bug. I tried it on a AthlonII-X4 
(which has maxleaf=5 as any newer AMD machines) and it showed the same 
bug. On Intel boxes this bug should trigger on every CPU starting with 
some Pentium4 models, including all Core chips.

Have you tried versions with a newer service pack (SP6)?


BTW: Does anyone knows what the problem with Windows95/98 on KVM is? I
tried some tracing today, but couldn't find a hint.


Um.  The bugreport(s) come as a surprize for me: I tried to
install win98 in kvm several times in the past but setup
always failed - different messages in different versions
of kvm, either unable to emulate or real mode trap or
something else, or just lockup, usually on first reboot.
So - the bugreports talks about mouse non-working, but
this means win98 itself works somehow...  I dunno :)
I think these bug reports are about plain QEMU. I tried it yesterday, in 
fact the mouse is non-functional. In KVM Windows95 gives me a black 
screen after the welcome screen with the moving bottom row. There are 
just two lines at the top: (translated from the german version)

While initializing device NTKERN:
Windows protection fault. Restart the computer.

KVM catched some #UDs due to ARPL from VM86 mode, but TCG got them too 
and it survived. So if anyone has some more hints, I'd be grateful.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

[Qemu-devel] [PATCH] resent: fix CPUID vendor override

[Andre Przywara]

the meaning of vendor_override is actually the opposite of how it
is currently used :-(
Fix it to allow KVM to export the non-native CPUID vendor if
explicitly requested by the user.
The semantic is now as intended:
- With TCG, the guest always sees the configured vendor.
- With KVM, the default is to propagate the host's vendor
  - when explicitly requested via -cpu base,vendor=xxx obey this
and use the specified vendor

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Hi,

this hasn't been picked up the last time I sent it out, are there any
objections?

Regards,
Andre.

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 56938e2..99d1f44 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -962,7 +962,7 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t 
*ebx,
  * this if you want to use KVM's sysenter/syscall emulation
  * in compatibility mode and when doing cross vendor migration
  */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled()  ! env-cpuid_vendor_override) {
 host_cpuid(0, 0, NULL, ebx, ecx, edx);
 }
 }
-- 
1.6.4

[Qemu-devel] [PATCH] resent: x86/cpuid: Add kvm32 CPU model

[Andre Przywara]

Create a kvm32 CPU model that describes a least common denominator
for KVM capable guest CPUs. Useful for migration purposes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index a80baa4..76b897d 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -363,6 +363,20 @@ static x86_def_t builtin_x86_defs[] = {
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
+.name = kvm32,
+.level = 5,
+.family = 15,
+.model = 6,
+.stepping = 1,
+.features = PPRO_FEATURES |
+CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
+.ext_features = CPUID_EXT_SSE3,
+.ext2_features = PPRO_FEATURES  EXT2_FEATURE_MASK,
+.ext3_features = 0,
+.xlevel = 0x8008,
+.model_id = Common 32-bit KVM processor
+},
+{
 .name = coreduo,
 .level = 10,
 .family = 6,
-- 
1.6.4

[Qemu-devel] [PATCH] resent: x86/cpuid: propagate further CPUID leafs when -cpu host

[Andre Przywara]

-cpu host currently only propagates the CPU's family/model/stepping,
the brand name and the feature bits.
Add a whitelist of safe CPUID leafs to let the guest see the actual
CPU's cache details and other things.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpu.h   |6 +-
 target-i386/cpuid.c |   45 +
 2 files changed, 42 insertions(+), 9 deletions(-)

Again this patch was not yet applied, without any discussion I can remember.
Please apply.

Thanks,
Andre.

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 548ab80..77cbbdb 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -581,6 +581,10 @@ typedef struct {
 
 #define NB_MMU_MODES 2
 
+#define CPUID_FLAGS_BUILTIN  (1  0)
+#define CPUID_FLAGS_VENDOR_OVERRIDE  (1  1)
+#define CPUID_FLAGS_HOST (1  2) 
+
 typedef struct CPUX86State {
 /* standard registers */
 target_ulong regs[CPU_NB_REGS];
@@ -685,7 +689,7 @@ typedef struct CPUX86State {
 uint32_t cpuid_ext2_features;
 uint32_t cpuid_ext3_features;
 uint32_t cpuid_apic_id;
-int cpuid_vendor_override;
+uint32_t cpuid_flags;
 
 /* MTRRs */
 uint64_t mtrr_fixed[11];
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 99d1f44..a80baa4 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -213,7 +213,6 @@ typedef struct x86_def_t {
 uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
 uint32_t xlevel;
 char model_id[48];
-int vendor_override;
 uint32_t flags;
 } x86_def_t;
 
@@ -489,7 +488,7 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 x86_cpu_def-ext2_features = edx;
 x86_cpu_def-ext3_features = ecx;
 cpu_x86_fill_model_id(x86_cpu_def-model_id);
-x86_cpu_def-vendor_override = 0;
+x86_cpu_def-flags = CPUID_FLAGS_HOST;
 
 return 0;
 }
@@ -633,7 +632,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 x86_cpu_def-vendor2 |= ((uint8_t)val[i + 4])  (8 * i);
 x86_cpu_def-vendor3 |= ((uint8_t)val[i + 8])  (8 * i);
 }
-x86_cpu_def-vendor_override = 1;
+x86_cpu_def-flags |= CPUID_FLAGS_VENDOR_OVERRIDE;
 } else if (!strcmp(featurestr, model_id)) {
 pstrcpy(x86_cpu_def-model_id, sizeof(x86_cpu_def-model_id),
 val);
@@ -731,7 +730,8 @@ void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, 
const char *fmt, ...),
 return;
 }
 for (def = x86_defs; def; def = def-next) {
-snprintf(buf, sizeof (buf), def-flags ? [%s]: %s, def-name);
+snprintf(buf, sizeof (buf),
+ def-flags  CPUID_FLAGS_BUILTIN ? [%s]: %s, def-name);
 if (model || dump) {
 (*cpu_fprintf)(f, x86 %16s  %-48s\n, buf, def-model_id);
 } else {
@@ -785,7 +785,7 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-cpuid_vendor2 = CPUID_VENDOR_INTEL_2;
 env-cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
 }
-env-cpuid_vendor_override = def-vendor_override;
+env-cpuid_flags = def-flags;
 env-cpuid_level = def-level;
 if (def-family  0x0f)
 env-cpuid_version = 0xf00 | ((def-family - 0x0f)  20);
@@ -941,7 +941,7 @@ void x86_cpudef_setup(void)
 
 for (i = 0; i  ARRAY_SIZE(builtin_x86_defs); ++i) {
 builtin_x86_defs[i].next = x86_defs;
-builtin_x86_defs[i].flags = 1;
+builtin_x86_defs[i].flags |= CPUID_FLAGS_BUILTIN;
 x86_defs = builtin_x86_defs[i];
 }
 #if !defined(CONFIG_USER_ONLY)
@@ -962,22 +962,51 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t 
*ebx,
  * this if you want to use KVM's sysenter/syscall emulation
  * in compatibility mode and when doing cross vendor migration
  */
-if (kvm_enabled()  ! env-cpuid_vendor_override) {
+if (kvm_enabled() 
+(env-cpuid_flags  CPUID_FLAGS_VENDOR_OVERRIDE) == 0) {
 host_cpuid(0, 0, NULL, ebx, ecx, edx);
 }
 }
 
+/* safe CPUID leafs to propagate to guest if -cpu host is specified
+ * Intel defined leafs:
+ * Cache descriptors (0x02)
+ * Deterministic cache parameters (0x04)
+ * Monitor/MWAIT parameters (0x05)
+ */
+#define CPUID_LEAF_PROPAGATE ((1  0x02) | (1  0x04) | (1  0x05))
+
+/* AMD defined leafs:
+ * L1 Cache and TLB (0x05)
+ * L2+L3 TLB (0x06)
+ * LongMode address size (0x08)
+ * 1GB page TLB (0x19)
+ * Performance optimization (0x1A)
+ */
+#define CPUID_LEAF_PROPAGATE_EXTENDED ((1  0x05) | (1  0x06) |\
+   (1  0x08) | (1  0x19) | (1  0x1A))
+
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
 {
-/* test if maximum index reached */
 if (index  0x8000) {
+/* test if maximum index reached */
 if (index  env

Re: [Qemu-devel] [PATCH] resent: x86/cpuid: propagate further CPUID leafs when -cpu host

[Andre Przywara]

Anthony Liguori wrote:

On 05/21/2010 02:50 AM, Andre Przywara wrote:

-cpu host currently only propagates the CPU's family/model/stepping,
the brand name and the feature bits.
Add a whitelist of safe CPUID leafs to let the guest see the actual
CPU's cache details and other things.

Signed-off-by: Andre Przywaraandre.przyw...@amd.com
   


The problem I can see is that this greatly increases the chances of 
problems with live migration since we don't migrate the cpuid state.
I think that should be fixed. Although -cpu host is not a wise choice 
for migration, even without these additional leaves the feature bits 
probably don't match between source and target.



What's the benefit of exposing this information to the guest?


That is mostly to propagate the cache size and organization parameters 
to the guest:

 +/* safe CPUID leafs to propagate to guest if -cpu host is specified
 + * Intel defined leafs:
 + * Cache descriptors (0x02)
 + * Deterministic cache parameters (0x04)
 + * Monitor/MWAIT parameters (0x05)
 + *
 + * AMD defined leafs:
 + * L1 Cache and TLB (0x05)
 + * L2+L3 TLB (0x06)
 + * LongMode address size (0x08)
 + * 1GB page TLB (0x19)
 + * Performance optimization (0x1A)
 + */
Since at least L1 and L2 caches are mostly private to vCPUs, I see no 
reason to disguise them.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

Re: [Qemu-devel] [PATCH] resent: x86/cpuid: propagate further CPUID leafs when -cpu host

[Andre Przywara]

Anthony Liguori wrote:

On 05/25/2010 08:21 AM, Andre Przywara wrote:

What's the benefit of exposing this information to the guest?
That is mostly to propagate the cache size and organization parameters 
to the guest:

+/* safe CPUID leafs to propagate to guest if -cpu host is specified
+ * Intel defined leafs:
+ * Cache descriptors (0x02)
+ * Deterministic cache parameters (0x04)
+ * Monitor/MWAIT parameters (0x05)
+ *
+ * AMD defined leafs:
+ * L1 Cache and TLB (0x05)
+ * L2+L3 TLB (0x06)
+ * LongMode address size (0x08)
+ * 1GB page TLB (0x19)
+ * Performance optimization (0x1A)
+ */
Since at least L1 and L2 caches are mostly private to vCPUs, I see no 
reason to disguise them.


But in practice, what is it useful for?  Just because we can expose it 
doesn't mean we should.
Beside the obvious high performance libraries (like the AMD ACML) also 
JVMs (and probably other managed code runtimes) use the cache 
information for optimization. Given the fact that we have a fairly large 
range of actual L2 cache sizes (from 256KB to 6MB on Intel) the fixed 
cache size that QEMU reports (1MB) is quite a bit off most of the times.
-cpu host is targeted to give the best performance to the user, with the 
drawback of missing or very limited migration experience. So we should 
expose as many features as possible.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

Re: [Qemu-devel] New Bitmap module ?

[Andre Przywara]

Corentin Chary wrote:

Hi,
I was working on merging VNC updates into bigger ones to see if it
lower the overhead (big updates sometime use less network/cpu than a
lot of small updates).
For that, I needed some new bitmap functions, and no we got, in vnc.c:
- set_bit
- clear_bit
- set_bits
- clear_bits
- find_next_bit
- find_next_zero_bit
- find_next_zero_area

Should we move that into bitmap.c/bitmap.h ?

Definitely! For my NUMA work I have also coded some bitmap functions.
Since mine are not performance critical, I reverted from implementing 
set_bits and clear_bits and replaced them with a loop in the calling 
code. So I could just could come around with a macro only 
implementation, for which a header file suffices (attached for reference).



Which part of QEMU could
use that (block.c maybe ?)
As mentioned, the yet to be submitted NUMA code would benefit from it. 
Linux has also bitmap code, maybe you could leverage this (if not 
already done).


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12
#ifndef __BITMAP_H__
#define __BITMAP_H__

#ifndef HOST_LONG_BITS
#define HOST_LONG_BITS (sizeof(long) * 8)
#endif

#define bitmap_isset(bm,bit) \
(!!bm[(bit) / HOST_LONG_BITS]  (1ULL  ((bit) % HOST_LONG_BITS)))
#define bitmap_set(bm,bit) \
(bm[(bit) / HOST_LONG_BITS] |= (1ULL  ((bit) % HOST_LONG_BITS)))
#define bitmap_unset(bm,bit) \
(bm[(bit) / HOST_LONG_BITS] = ~(1ULL  ((bit) % HOST_LONG_BITS)))
#define DECLARE_BITMAP(bm,len) \
unsigned long bm[((len) + HOST_LONG_BITS - 1) / HOST_LONG_BITS]
#define bitmap_clear(bm,len) \
memset(bm, 0, (len + 7) / 8)
#define bitmap_fill(bm,len) \
memset(bm, 0xFF, (len + 7) / 8)

#endif

[Qemu-devel] Using Linux's CPUSET for KVM VCPUs

[Andre Przywara]

Hi all,

while working on NUMA host pinning, I experimented with vCPU affinity 
within QEMU, but left it alone as it would complicate the code and would 
not achieve better experience than using taskset with the monitor 
provided thread ids like it is done currently. During that I looked at 
Linux' CPUSET implementation 
(/src/linux-2.6/Documentation/cgroups/cpusets.txt).
In brief, this is a pseudo file system based, truly hierarchical 
implementation of restricting a set of processes (or threads, it uses 
PIDs) to a certain subset of the machine.
Sadly we cannot leverage this for true guest NUMA memory assignment, but 
it would work nice for pinning (or not) guest vCPUs. I had the following 
structure in mind:
For each guest there is a new CPUSET (mkdir $CPUSET_MNT/`cat 
/proc/$$/cpuset`/kvm_$guestname). One could then assign the guest global 
resources to this CPUSET.
For each vCPU there is a separate CPUSET located under this guest global 
one. This would allow for easy manipulation of the pinning of vCPUs, 
even from the console without any mgt app (although this could be easily 
implemented in libvirt).


/
|
+--/ kvm_guest_01
|  |
|  +-- VCPU0
|  |
|  +-- VCPU1
|
+--/ kvm_guest_02
...

What do you think about it? It is worth implementing this?

Regards,
Andre.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Andre Przywara]

Anthony Liguori wrote:

Gleb Natapov wrote:

I thought KVM emulates the syscall instruction?  I swear I've seen
patches for that.



It is. Starting from 2.6.32.
  


Okay, so this is a performance vs. migration compatibility issue then?

BTW, couldn't we just not advertise syscall in cpuid?

Looks like a good idea, but at least Linux does not care ;-)
In 64-on-64 bit mode syscall is unconditionally used, which is OK, since 
it is architectural to AMD64. Intel integrated this, too.
In 32bit legacy mode AMD supports both syscall and sysenter, but mostly 
sysenter is used when the SEP CPUID bit is on. Intel does not support 
syscall, but this also is not advertised when CPUID is executed in 32bit.
In 32-on-64 compat mode the support is exclusive: AMD does not support 
sysenter, Intel does not support syscall. Since the AMD CPUID 
advertising is (at least) a bit unfortunate here ;-), Linux does not 
care about the bits, but unconditionally uses syscall on AMD and 
sysenter on Intel. See arch/x86/kernel/cpu/intel.c:early_init_intel():

#ifdef CONFIG_X86_64
set_cpu_cap(c, X86_FEATURE_SYSENTER32);
#else
A similiar line is in cpu/amd.c:early_init_amd().
Then in arch/x86/vdso/vdso32-setup.c:sysenter_setup() these bit are used 
to populate the VDSO page. In the whole process CPUID feature bits are 
not honored, so you cannot get around it.
The only solution would be to inject neither AMD or Intel (I used 
VirtualLinux) in the past. This works fine for Linux (even better than 
the other two, since no vendor-specific workarounds are triggered), but 
64bit Windows refuses to boot on anything other than AuthenticAMD, 
GenuineIntel or CentaurHauls. Linux would then use int80h, which is 
slower than syscall or sysenter, but still much faster than emulation.


So given the whole mess I'd recommend to copy over Avi's propagate 
local vendor solution, as this solves the problem for 99.9+% of the 
cases. The remaining cases are cross-vendor migration, where we are fine 
with the 2.6.32 syscall/sysenter emulation. Although this is not as slow 
as one thinks (I measured less than 5% slowdown in kernbench), I'd not 
recommend it for common usage except cross-vendor migration.


BTW: Windows is not affected at all with this compat mode issue, because 
32bit applications switch to 64bit mode before doing the actual system 
call (which uses the 64bit syscall instruction).


That should fix 
it too without sacrificing migration compatibility.  We get a slight 
slowdown on AMD hosts
I think there would be no slowdown. At least Linux only cares about this 
bit if in legacy 32bit mode, but here it uses sysenter anyway if this is 
present:

From arch/x86/vdso/vdso32-setup.c:
#else  /* CONFIG_X86_32 */
#define vdso32_sysenter()   (boot_cpu_has(X86_FEATURE_SEP))
#define vdso32_syscall()(0)

but that's probably minor compared to the cost of 
using emulated syscall on Intel hosts.



Regards,
Andre.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[Qemu-devel] [PATCH] osdep: Fix runtime failure on older Linux kernels

[Andre Przywara]

If QEMU finds newer kernel header files on compilation time, it will use
advertised features like pipe2 or SOCK_CLOEXEC by just doing a compile test.
If later the executables are executed on an older kernel (2.6.27,
like Xen Dom0 2.6.18), then QEMU will fail on opening sockets and creating
pipes and returns the rather unspecific qemu_init_main_loop failed.
This patch fixes this by checking the return values of these calls
for EINVAL and ENOSYS and falling back to the older versions automatically.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 osdep.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/osdep.c b/osdep.c
index 7509c5b..9949606 100644
--- a/osdep.c
+++ b/osdep.c
@@ -262,13 +262,15 @@ int qemu_pipe(int pipefd[2])
 
 #ifdef CONFIG_PIPE2
 ret = pipe2(pipefd, O_CLOEXEC);
-#else
+if (ret != -1 || errno != ENOSYS) {
+return ret;
+}
+#endif
 ret = pipe(pipefd);
 if (ret == 0) {
 qemu_set_cloexec(pipefd[0]);
 qemu_set_cloexec(pipefd[1]);
 }
-#endif
 
 return ret;
 }
@@ -283,12 +285,14 @@ int qemu_socket(int domain, int type, int protocol)
 
 #ifdef SOCK_CLOEXEC
 ret = socket(domain, type | SOCK_CLOEXEC, protocol);
-#else
+if (ret != -1 || errno != EINVAL) {
+return ret;
+}
+#endif
 ret = socket(domain, type, protocol);
 if (ret = 0) {
 qemu_set_cloexec(ret);
 }
-#endif
 
 return ret;
 }
@@ -302,12 +306,14 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
 
 #ifdef CONFIG_ACCEPT4
 ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-#else
+if (ret != -1 || errno != EINVAL) {
+return ret;
+}
+#endif
 ret = accept(s, addr, addrlen);
 if (ret = 0) {
 qemu_set_cloexec(ret);
 }
-#endif
 
 return ret;
 }
-- 
1.6.4

Re: [Qemu-devel] [PATCH] osdep: Fix runtime failure on older Linux kernels

[Andre Przywara]

Kevin Wolf wrote:


@@ -302,12 +306,14 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
 
 #ifdef CONFIG_ACCEPT4

 ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-#else
+if (ret != -1 || errno != EINVAL) {


Shouldn't this be an ENOSYS?
Oh, right you are. This was the untested part, because my glibc didn't 
have accept4. I only saw the SOCK_CLOEXEC name on this...

Fixed patch follows...

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

[Qemu-devel] [PATCH v2] osdep: Fix runtime failure on older Linux kernels

[Andre Przywara]

If QEMU finds newer kernel header files on compilation time, it will use
advertised features like pipe2 or SOCK_CLOEXEC by just doing a compile test.
If later the executables are executed on an older kernel (2.6.27,
like Xen Dom0 2.6.18), then QEMU will fail on opening sockets and creating
pipes and returns the rather unspecific qemu_init_main_loop failed.
This patch fixes this by checking the return values of these calls
for EINVAL and ENOSYS and falling back to the older versions automatically.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 osdep.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

v2 changed the errno check from EINVAL to ENOSYS for accept4

diff --git a/osdep.c b/osdep.c
index 7509c5b..60469ed 100644
--- a/osdep.c
+++ b/osdep.c
@@ -262,13 +262,15 @@ int qemu_pipe(int pipefd[2])
 
 #ifdef CONFIG_PIPE2
 ret = pipe2(pipefd, O_CLOEXEC);
-#else
+if (ret != -1 || errno != ENOSYS) {
+return ret;
+}
+#endif
 ret = pipe(pipefd);
 if (ret == 0) {
 qemu_set_cloexec(pipefd[0]);
 qemu_set_cloexec(pipefd[1]);
 }
-#endif
 
 return ret;
 }
@@ -283,12 +285,14 @@ int qemu_socket(int domain, int type, int protocol)
 
 #ifdef SOCK_CLOEXEC
 ret = socket(domain, type | SOCK_CLOEXEC, protocol);
-#else
+if (ret != -1 || errno != EINVAL) {
+return ret;
+}
+#endif
 ret = socket(domain, type, protocol);
 if (ret = 0) {
 qemu_set_cloexec(ret);
 }
-#endif
 
 return ret;
 }
@@ -302,12 +306,14 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
 
 #ifdef CONFIG_ACCEPT4
 ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-#else
+if (ret != -1 || errno != ENOSYS) {
+return ret;
+}
+#endif
 ret = accept(s, addr, addrlen);
 if (ret = 0) {
 qemu_set_cloexec(ret);
 }
-#endif
 
 return ret;
 }
-- 
1.6.4

Re: [Qemu-devel] Re: SVM support in 0.12?

[Andre Przywara]

Jun Koi wrote:

I am running latest Qemu 0.12-rc. My guest VM runs Linux kernel 2.6.31.

Because Qemu now supports SVM, I expect to see the SVM flag in
/proc/cpuinfo, but that is not the case.

So it seems SVM support is not enabled by default configuration??


My host and guest are both 32 bit Linux, if that matters.

(And this is pure Qemu, without using KVM or KQemu)

Kqemu actually works with svm emulation.


...

qemu -m 500 -cpu qemu32,+svm -cdrom ubuntu.iso

I verified that /proc/cpuinfo has no svm flag. So SVM doesnt work on 32bit
host.

I will try that with 64bit host to see how it goes.

The host doesn't matter. You can easily run qemu-system-x86_64 on a 32-bit
host.



I can confirm that SVM works well on x86-64 target, but fails on i386 target.


The Linux kernel will only detect SVM if the machine is AMD (see 
linux-2.6/arch/x86/include/asm/virtext.h:cpu_has_svm())

So please try:
$ qemu -m 500 -cpu qemu32,+svm,vendor=AuthenticAMD -cdrom ubuntu.iso
(because the default vendor for qemu32 is Intel, for qemu64 AMD)

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [Qemu-devel] Re: SVM support in 0.12?

[Andre Przywara]

Andre Przywara wrote:

Jun Koi wrote:
I am running latest Qemu 0.12-rc. My guest VM runs Linux kernel 
2.6.31.


Because Qemu now supports SVM, I expect to see the SVM flag in
/proc/cpuinfo, but that is not the case.

So it seems SVM support is not enabled by default configuration??


My host and guest are both 32 bit Linux, if that matters.

(And this is pure Qemu, without using KVM or KQemu)

Kqemu actually works with svm emulation.


...

qemu -m 500 -cpu qemu32,+svm -cdrom ubuntu.iso

I verified that /proc/cpuinfo has no svm flag. So SVM doesnt work on 
32bit

host.

I will try that with 64bit host to see how it goes.
The host doesn't matter. You can easily run qemu-system-x86_64 on a 
32-bit

host.



I can confirm that SVM works well on x86-64 target, but fails on i386 
target.


The Linux kernel will only detect SVM if the machine is AMD (see 
linux-2.6/arch/x86/include/asm/virtext.h:cpu_has_svm())

So please try:
$ qemu -m 500 -cpu qemu32,+svm,vendor=AuthenticAMD -cdrom ubuntu.iso
(because the default vendor for qemu32 is Intel, for qemu64 AMD)


Should have checked this before the post ;-):
qemu32 has a xlevel of 0, so no AMD-defined CPUID leafs will be parsed.
Either fix this explicitly with xlevel=0xa or use athlon as your base 
CPU model:

$ qemu -m 500 -cpu athlon,+svm -cdrom ubuntu.iso

This made my Linux show the SVM flag.

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Andre Przywara]

Avi Kivity wrote:

On 12/20/2009 07:59 PM, Anthony Liguori wrote:

Gleb Natapov wrote:
Windows is a mystery box, so we can speculate as much as we want 
about it.

If you don't like something just say it may break Windows :) Losing
activation does sound like an issue too.


Downgrading seems more likely to cause problems.  Considering running 
mplayer in a guest.  If it detects SSE3 in one host and migrate to 
another host that doesn't have SSE3, you'll be running an instruction 
stream that uses instructions the processor doesn't support resulting 
in the application faulting due to an illegal operation.




Migration needs preparation beforehand.  You can't take two random hosts 
with two random qemu flag sets and expect it to work.


KVM's cpuid filter doesn't help here because it won't attempt to mask 
things like sse3.  It would be insane to emulate sse3 too.


It does expose sse3 support (called pni in Linux IIRC).  Not sure if 
qemu masks it, but the information is there.

What about using -cpu kvm64?
This has SSE3 as well as CMPXCHG16, both are available in all VMX/SVM 
capable machines (as far as I could find out) and in TCG.
This gives a pretty descent base without loosing many relevant 
performance features.


... 
This is a classic management tool problem and the solution usually is 
a set of heuristics depending on how conservative the target audience is.


Right.  My concern is with casual users upgrading their machine, not 
enterprise users who should be protected by their tools.
Then what about fixing the CPUID bits to those returned by the KVM 
module the first time the guest is started? Later you would only use 
those bits (which may have been cropped) for later restarts of the same 
guest. If you only upgrade your machine, then there should be no problems.


Regards,
Andre.

P.S. What feature bits do we talk about?
Maybe we should group them and use aliases for those.
SS_S_E3 and SSE4.x come to mind, are any other bits really relevant? (Or 
do they justify the pain we have when propagating them?)

Then we would only need: Athlon64, Core2, Nehalem (and maybe Phenom).

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..

[Andre Przywara]

john cooper wrote:

Chris Wright wrote:

* Daniel P. Berrange (berra...@redhat.com) wrote:

To be honest all possible naming schemes for '-cpu name' are just as
unfriendly as each other. The only user friendly option is '-cpu host'. 


IMHO, we should just pick a concise naming scheme  document it. Given
they are all equally unfriendly, the one that has consistency with vmware
naming seems like a mild winner.

Heh, I completely agree, and was just saying the same thing to John
earlier today.  May as well be -cpu {foo,bar,baz} since the meaning for
those command line options must be well-documented in the man page.


I can appreciate the concern of wanting to get this
as correct as possible.  But ultimately we just
need three unique tags which ideally have some relation
to their associated architectures.  The diatribes
available from /proc/cpuinfo while generally accurate
don't really offer any more of a clue to the model
group, and in their unmodified form are rather unwieldy
as command line flags.
I agree. I'd underline that this patch is for migration purposes only, 
so you don't want to specify an exact CPU, but more like a class of 
CPUs. If you look into the available CPUID features in each CPU, you 
will find that there are only a few groups, with currently three for 
each vendor being a good guess.
/proc/cpuinfo just prints out marketing names, which have only a mild 
relationship to a feature-related technical CPU model. Maybe we can use 
a generation approach like the AMD Opteron ones for Intel, too.

These G1/G2/G3 names are just arbitrary and have no roots within AMD.

I think that an exact CPU model specification is out of scope for this 
patch and maybe even for QEMU. One could create a database with CPU 
names and associated CPUID flags and provide an external tool to 
generate a QEMU command line out of this. Keeping this database 
up-to-date (especially for desktop CPU models) is a burden that the QEMU 
project does not want to bear.





This is from an EVC kb article[1]:


Here is a pointer to a more detailed version:

   
http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1003212


We probably should also add an option to dump out the
full set of qemu-side cpuid flags for the benefit of
users and upper level tools.

You mean like this one?
http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg01228.html
Resending this patch set is on my plan for next week. What is the state 
of this patch? Will it go in soon? Then I'd rebase my patch set on top 
of it.


Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

[Qemu-devel] [PATCH 00/13] i386 cpuid: cleanup and fixes

[Andre Przywara]

Hi,

first: I know that this conflicts with John Cooper's latest patch, but I want
to send this out for review and to help merging the stuff.

This patchset cleans up the CPUID handling code in QEMU. The biggest change
is obviously the move of the CPUID function to a separate file (cpuid.c).
This helps to split up a rather large source file, which's name (helper.c) is
also a bit misleading.
Please tell me soon if you don't like it so that I can rebase the rest of
patches.
Additionally the rest of the patches beautifies or simplifies some code.
Feature additions are:
 5/13: add missing CPUID feature bit names
 6/13: list CPUID feature bit names when using -cpu ?
 9/13: -cpu host propagates more CPUID leafs, so that the cache topology
   will be visibile in the guest
10/13: add CPUID feature bit trimming for TCG: Features not supported by
   the emulator will be masked out.
11/13: always show all CPU types: also expose the newer (64bit) CPU types
   for the i386 emulator. 64bit features will be masked out due to 10/13.
12/13: add kvm32 CPU model: Per popular request add a counterpart to kvm64
   describing a basic hardware virtualization capable CPU for migration
   purposes.

More details in the commit messages.

Note: In opposite to the last version I left out patches which change
the CPUID bits of existing CPU models to avoid regressions with guests.

Please review and comment.

Regards,
Andre.

[Qemu-devel] [PATCH 02/13] cpuid: replace magic number with named constant

[Andre Przywara]

CPUID leaf Fn8000_0001.EDX contains a copy of many Fn_0001.EDX bits.
Define a name for the mask to improve readability and avoid typos.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index aaa14ba..0a17020 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -130,6 +130,7 @@ typedef struct x86_def_t {
   CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
   CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
+#define EXT2_FEATURE_MASK 0x0183F3FF
 static x86_def_t x86_defs[] = {
 #ifdef TARGET_X86_64
 {
@@ -147,7 +148,7 @@ static x86_def_t x86_defs[] = {
 /* this feature is needed for Solaris and isn't fully implemented */
 CPUID_PSE36,
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
 .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
 CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
@@ -170,7 +171,7 @@ static x86_def_t x86_defs[] = {
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
 CPUID_EXT_POPCNT,
 /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
 CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
 CPUID_EXT2_FFXSR,
@@ -220,7 +221,7 @@ static x86_def_t x86_defs[] = {
 /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16,
 /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) |
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) |
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
 /* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
 CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
@@ -308,7 +309,7 @@ static x86_def_t x86_defs[] = {
 .stepping = 3,
 .features = PPRO_FEATURES | CPUID_PSE36 | CPUID_VME |
 CPUID_MTRR | CPUID_MCA,
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | CPUID_EXT2_MMXEXT |
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
CPUID_EXT2_MMXEXT |
   CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
 .xlevel = 0x8008,
 /* XXX: put another string ? */
@@ -330,7 +331,7 @@ static x86_def_t x86_defs[] = {
 CPUID_EXT_SSE3 /* PNI */ | CPUID_EXT_SSSE3,
 /* Missing: CPUID_EXT_DSCPL | CPUID_EXT_EST |
  * CPUID_EXT_TM2 | CPUID_EXT_XTPR */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | CPUID_EXT2_NX,
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | CPUID_EXT2_NX,
 /* Missing: .ext3_features = CPUID_EXT3_LAHF_LM */
 .xlevel = 0x800A,
 .model_id = Intel(R) Atom(TM) CPU N270   @ 1.60GHz,
-- 
1.6.4

[Qemu-devel] [PATCH 05/13] cpuid: add missing CPUID feature flag names

[Andre Przywara]

Some CPUID feature flags had no string value, so they could not be
switched on or off from the command line.
Add names for the missing ones mentioned in the current public CPUID
specification from both Intel and AMD. Those only mentioned in the
Linux kernel source I put as comments.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0238718..19d58e1 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -52,11 +52,11 @@ static const char *feature_name[] = {
 fxsr, sse, sse2, ss, ht /* Intel htt */, tm, ia64, pbe,
 };
 static const char *ext_feature_name[] = {
-pni /* Intel,AMD sse3 */, NULL, NULL, monitor,
-ds_cpl, vmx, NULL /* Linux smx */, est,
-tm2, ssse3, cid, NULL, NULL, cx16, xtpr, NULL,
-NULL, NULL, dca, NULL, NULL, NULL, NULL, popcnt,
-NULL, NULL, NULL, NULL, NULL, NULL, NULL, hypervisor,
+pni /* Intel,AMD sse3 */, pclmuldq, dtes64, monitor,
+ds_cpl, vmx, smx, est,
+tm2, ssse3, cid, NULL, NULL /* FMA */, cx16, xtpr, pdcm,
+NULL, NULL, dca, sse4_1, sse4_2, x2apic, movbe, popcnt,
+NULL, aes, xsave, osxsave, NULL /* AVX */, NULL, NULL, hypervisor,
 };
 static const char *ext2_feature_name[] = {
 fpu, vme, de, pse, tsc, msr, pae, mce,
@@ -71,8 +71,9 @@ static const char *ext3_feature_name[] = {
 lahf_lm /* AMD LahfSahf */, cmp_legacy,
 svm, extapic /* AMD ExtApicSpace */,
 cr8legacy /* AMD AltMovCr8 */, abm, sse4a, misalignsse,
-3dnowprefetch, osvw, NULL /* Linux ibs */, NULL, skinit, wdt, 
NULL, NULL,
-NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+3dnowprefetch, osvw, ibs, NULL /* SSE-5 */,
+skinit, wdt, NULL, NULL,
+NULL, NULL, NULL, nodeid_msr, NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
-- 
1.6.4

[Qemu-devel] [PATCH 08/13] cpuid: simplify CPUID flag search function

[Andre Przywara]

avoid code duplication and handle the CPUID flag name search in a
loop.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   38 +-
 1 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 3f56c50..635c2f4 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -90,34 +90,22 @@ static void add_flagname_to_bitmaps(const char *flagname, 
uint32_t *features,
 uint32_t *ext3_features,
 uint32_t *kvm_features)
 {
-int i;
+int i, j;
 int found = 0;
-
-for ( i = 0 ; i  32 ; i++ )
-if (feature_name[i]  !strcmp (flagname, feature_name[i])) {
-*features |= 1  i;
-found = 1;
-}
-for ( i = 0 ; i  32 ; i++ )
-if (ext_feature_name[i]  !strcmp (flagname, ext_feature_name[i])) {
-*ext_features |= 1  i;
-found = 1;
-}
-for ( i = 0 ; i  32 ; i++ )
-if (ext2_feature_name[i]  !strcmp (flagname, ext2_feature_name[i])) {
-*ext2_features |= 1  i;
-found = 1;
-}
-for ( i = 0 ; i  32 ; i++ )
-if (ext3_feature_name[i]  !strcmp (flagname, ext3_feature_name[i])) {
-*ext3_features |= 1  i;
-found = 1;
-}
-for ( i = 0 ; i  32 ; i++ )
-if (kvm_feature_name[i]  !strcmp (flagname, kvm_feature_name[i])) {
-*kvm_features |= 1  i;
-found = 1;
+const char ** feature_names[5] = {feature_name, ext_feature_name,
+  ext2_feature_name, ext3_feature_name,
+  kvm_feature_name};
+uint32_t* feature_flags[5] = {features, ext_features, ext2_features,
+  ext3_features, kvm_features};
+
+for (j = 0; j  5; j++) {
+for ( i = 0 ; i  32 ; i++ ) {
+if (feature_names[j][i]  !strcmp(flagname, feature_names[j][i])) 
{
+*feature_flags[j] |= 1  i;
+found = 1;
+}
 }
+}
 
 if (!found) {
 fprintf(stderr, CPU feature %s not found\n, flagname);
-- 
1.6.4

[Qemu-devel] [PATCH 11/13] cpuid: Always expose 32 and 64-bit CPUs

[Andre Przywara]

Since 64-bit capability is just another CPUID bit we now properly
mask, there is no reason anymore to hide the 64-bit capable CPU
models from a 32-bit only QEMU. All 64-bit CPUs can be used
perfectly in 32-bit legacy mode anyway, so these models also make
sense for 32-bit.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 6e6ee54..b03a363 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -153,7 +153,6 @@ typedef struct x86_def_t {
   CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
 
 static x86_def_t x86_defs[] = {
-#ifdef TARGET_X86_64
 {
 .name = qemu64,
 .level = 4,
@@ -252,7 +251,6 @@ static x86_def_t x86_defs[] = {
 .xlevel = 0x8008,
 .model_id = Common KVM processor
 },
-#endif
 {
 .name = qemu32,
 .level = 4,
-- 
1.6.4

[Qemu-devel] [PATCH 10/13] cpuid: add TCG feature bit trimming

[Andre Przywara]

In KVM we trim the user provided CPUID bits to match the host CPU's
one. Introduce a similar feature to QEMU/TCG. Create a mask of TCG's
capabilities and apply it to the user bits.
This allows to let the CPU models reflect their native archetypes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   26 ++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 6aa1f3f..6e6ee54 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -137,6 +137,21 @@ typedef struct x86_def_t {
   CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
 #define EXT2_FEATURE_MASK 0x0183F3FF
+
+#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
+  CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
+  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+  CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
+  CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
+#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | \
+  CPUID_EXT_CX16 | CPUID_EXT_POPCNT | CPUID_EXT_XSAVE | \
+  CPUID_EXT_HYPERVISOR)
+#define TCG_EXT2_FEATURES ((TCG_FEATURES  EXT2_FEATURE_MASK) | \
+  CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
+  CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT)
+#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
+  CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+
 static x86_def_t x86_defs[] = {
 #ifdef TARGET_X86_64
 {
@@ -616,6 +631,17 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-cpuid_ext2_features = def-ext2_features;
 env-cpuid_xlevel = def-xlevel;
 env-cpuid_kvm_features = def-kvm_features;
+env-cpuid_ext3_features = def-ext3_features;
+if (!kvm_enabled()) {
+env-cpuid_features = TCG_FEATURES;
+env-cpuid_ext_features = TCG_EXT_FEATURES;
+env-cpuid_ext2_features = (TCG_EXT2_FEATURES
+#ifdef TARGET_X86_64
+| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
+#endif
+);
+env-cpuid_ext3_features = TCG_EXT3_FEATURES;
+}
 {
 const char *model_id = def-model_id;
 int c, len, i;
-- 
1.6.4

[Qemu-devel] [PATCH 03/13] cpuid: moved host_cpuid function and remove prototype

[Andre Przywara]

the host_cpuid function was located at the end of the file and had
a prototype before it's first use. Move it up and remove the
prototype.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   70 --
 1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0a17020..cc080f4 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -338,8 +338,40 @@ static x86_def_t x86_defs[] = {
 },
 };
 
-static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
-   uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+static void host_cpuid(uint32_t function, uint32_t count,
+   uint32_t *eax, uint32_t *ebx,
+   uint32_t *ecx, uint32_t *edx)
+{
+#if defined(CONFIG_KVM)
+uint32_t vec[4];
+
+#ifdef __x86_64__
+asm volatile(cpuid
+ : =a(vec[0]), =b(vec[1]),
+   =c(vec[2]), =d(vec[3])
+ : 0(function), c(count) : cc);
+#else
+asm volatile(pusha \n\t
+ cpuid \n\t
+ mov %%eax, 0(%2) \n\t
+ mov %%ebx, 4(%2) \n\t
+ mov %%ecx, 8(%2) \n\t
+ mov %%edx, 12(%2) \n\t
+ popa
+ : : a(function), c(count), S(vec)
+ : memory, cc);
+#endif
+
+if (eax)
+   *eax = vec[0];
+if (ebx)
+   *ebx = vec[1];
+if (ecx)
+   *ecx = vec[2];
+if (edx)
+   *edx = vec[3];
+#endif
+}
 
 static int cpu_x86_fill_model_id(char *str)
 {
@@ -578,40 +610,6 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 return 0;
 }
 
-static void host_cpuid(uint32_t function, uint32_t count,
-   uint32_t *eax, uint32_t *ebx,
-   uint32_t *ecx, uint32_t *edx)
-{
-#if defined(CONFIG_KVM)
-uint32_t vec[4];
-
-#ifdef __x86_64__
-asm volatile(cpuid
- : =a(vec[0]), =b(vec[1]),
-   =c(vec[2]), =d(vec[3])
- : 0(function), c(count) : cc);
-#else
-asm volatile(pusha \n\t
- cpuid \n\t
- mov %%eax, 0(%2) \n\t
- mov %%ebx, 4(%2) \n\t
- mov %%ecx, 8(%2) \n\t
- mov %%edx, 12(%2) \n\t
- popa
- : : a(function), c(count), S(vec)
- : memory, cc);
-#endif
-
-if (eax)
-   *eax = vec[0];
-if (ebx)
-   *ebx = vec[1];
-if (ecx)
-   *ecx = vec[2];
-if (edx)
-   *edx = vec[3];
-#endif
-}
 
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
  uint32_t *ecx, uint32_t *edx)
-- 
1.6.4

[Qemu-devel] [PATCH 09/13] cpuid: propagate further CPUID leafs when -cpu host

[Andre Przywara]

-cpu host currently only propagates the CPU's family/model/stepping,
the brand name and the feature bits.
Add a whitelist of safe CPUID leafs to let the guest see the actual
CPU's cache details and other things.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpu.h   |5 -
 target-i386/cpuid.c |   28 ++--
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f826d3d..982f815 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -581,6 +581,9 @@ typedef struct {
 
 #define NB_MMU_MODES 2
 
+#define CPUID_FLAGS_VENDOR_OVERRIDE 1
+#define CPUID_FLAGS_HOST 2
+
 typedef struct CPUX86State {
 /* standard registers */
 target_ulong regs[CPU_NB_REGS];
@@ -685,7 +688,7 @@ typedef struct CPUX86State {
 uint32_t cpuid_ext2_features;
 uint32_t cpuid_ext3_features;
 uint32_t cpuid_apic_id;
-int cpuid_vendor_override;
+uint32_t cpuid_flags;
 
 /* MTRRs */
 uint64_t mtrr_fixed[11];
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 635c2f4..6aa1f3f 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -122,7 +122,7 @@ typedef struct x86_def_t {
 uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
 uint32_t xlevel;
 char model_id[48];
-int vendor_override;
+uint32_t flags;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -419,7 +419,7 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 x86_cpu_def-ext2_features = edx;
 x86_cpu_def-ext3_features = ecx;
 cpu_x86_fill_model_id(x86_cpu_def-model_id);
-x86_cpu_def-vendor_override = 0;
+x86_cpu_def-flags = CPUID_FLAGS_HOST;
 
 return 0;
 }
@@ -529,7 +529,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 x86_cpu_def-vendor2 |= ((uint8_t)val[i + 4])  (8 * i);
 x86_cpu_def-vendor3 |= ((uint8_t)val[i + 8])  (8 * i);
 }
-x86_cpu_def-vendor_override = 1;
+x86_cpu_def-flags |= CPUID_FLAGS_VENDOR_OVERRIDE;
 } else if (!strcmp(featurestr, model_id)) {
 pstrcpy(x86_cpu_def-model_id, sizeof(x86_cpu_def-model_id),
 val);
@@ -602,7 +602,7 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-cpuid_vendor2 = CPUID_VENDOR_INTEL_2;
 env-cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
 }
-env-cpuid_vendor_override = def-vendor_override;
+env-cpuid_flags = def-flags;
 env-cpuid_level = def-level;
 if (def-family  0x0f)
 env-cpuid_version = 0xf00 | ((def-family - 0x0f)  20);
@@ -647,22 +647,38 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t 
*ebx,
  * this if you want to use KVM's sysenter/syscall emulation
  * in compatibility mode and when doing cross vendor migration
  */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled() 
+(env-cpuid_flags  CPUID_FLAGS_VENDOR_OVERRIDE) == 0) {
 host_cpuid(0, 0, NULL, ebx, ecx, edx);
 }
 }
 
+#define CPUID_LEAF_PROPAGATE ((1  0x02) | (1  0x04) | (1  0x05) |\
+  (1  0x0D))
+#define CPUID_LEAF_PROPAGATE_EXTENDED ((1  0x05) | (1  0x06) |\
+   (1  0x08) | (1  0x19) | (1  0x1A))
+
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
 {
-/* test if maximum index reached */
 if (index  0x8000) {
+/* test if maximum index reached */
 if (index  env-cpuid_xlevel)
 index = env-cpuid_level;
+if ((env-cpuid_flags  CPUID_FLAGS_HOST) 
+((1  (index - 0x8000))  CPUID_LEAF_PROPAGATE_EXTENDED)) {
+host_cpuid(index, count, eax, ebx, ecx, edx);
+return;
+}
 } else {
 if (index  env-cpuid_level)
 index = env-cpuid_level;
+if ((env-cpuid_flags  CPUID_FLAGS_HOST) 
+((1  index)  CPUID_LEAF_PROPAGATE)) {
+host_cpuid(index, count, eax, ebx, ecx, edx);
+return;
+}
 }
 
 switch(index) {
-- 
1.6.4

[Qemu-devel] [PATCH 04/13] cpuid: Replace strtok with get_opt_name

[Andre Przywara]

To avoid the non-reentrant capable strtok() use the QEMU defined
get_opt_name() to parse the -cpu parameter list. Since there is a
name clash between linux-user/mmap.c:qemu_malloc() and
qemu-malloc.c:qemu_malloc() I copied the small function from
qemu-option.c into cpuid.c. Not the best solution, bit IMO the
least intrusive and smallest one.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   34 --
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index cc080f4..0238718 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -24,6 +24,23 @@
 #include cpu.h
 #include kvm.h
 
+static const char *get_opt_name(char *buf, int buf_size,
+const char *p, char delim)
+{
+char *q;
+
+q = buf;
+while (*p != '\0'  *p != delim) {
+if (q  (q - buf)  buf_size - 1)
+*q++ = *p;
+p++;
+}
+if (q)
+*q = '\0';
+
+return p;
+}
+
 /* feature flags taken from Intel Processor Identification and the CPUID
  * Instruction and AMD's CPUID Specification. In cases of disagreement
  * about feature names, the Linux name is used. */
@@ -423,8 +440,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 unsigned int i;
 x86_def_t *def;
 
-char *s = strdup(cpu_model);
-char *featurestr, *name = strtok(s, ,);
+const char* s;
+char featurestr[64];
 uint32_t plus_features = 0, plus_ext_features = 0,
 plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0;
 uint32_t minus_features = 0, minus_ext_features = 0,
@@ -432,14 +449,15 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 minus_kvm_features = 0;
 uint32_t numvalue;
 
+s = get_opt_name(featurestr, 64, cpu_model, ',');
 def = NULL;
 for (i = 0; i  ARRAY_SIZE(x86_defs); i++) {
-if (strcmp(name, x86_defs[i].name) == 0) {
+if (strcmp(featurestr, x86_defs[i].name) == 0) {
 def = x86_defs[i];
 break;
 }
 }
-if (kvm_enabled()  strcmp(name, host) == 0) {
+if (kvm_enabled()  strcmp(featurestr, host) == 0) {
 cpu_x86_fill_host(x86_cpu_def);
 } else if (!def) {
 goto error;
@@ -453,10 +471,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 plus_ext_features, plus_ext2_features, plus_ext3_features,
 plus_kvm_features);
 
-featurestr = strtok(NULL, ,);
-
-while (featurestr) {
+while (*s != 0) {
 char *val;
+s = get_opt_name(featurestr, 64, s + 1, ',');
 if (featurestr[0] == '+') {
 add_flagname_to_bitmaps(featurestr + 1, plus_features,
 plus_ext_features, plus_ext2_features,
@@ -536,7 +553,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 (+feature|-feature|feature=xyz)\n, featurestr);
 goto error;
 }
-featurestr = strtok(NULL, ,);
 }
 x86_cpu_def-features |= plus_features;
 x86_cpu_def-ext_features |= plus_ext_features;
@@ -548,11 +564,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 x86_cpu_def-ext2_features = ~minus_ext2_features;
 x86_cpu_def-ext3_features = ~minus_ext3_features;
 x86_cpu_def-kvm_features = ~minus_kvm_features;
-free(s);
 return 0;
 
 error:
-free(s);
 return -1;
 }
 
-- 
1.6.4

[Qemu-devel] [PATCH 07/13] cpuid: remove unnecessary kvm_trim function

[Andre Przywara]

Correct me if I am wrong, but kvm_trim looks like a really bloated
implementation of a bitwise AND. So remove this function and replace
it with the real stuff(TM).

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/kvm.c |   27 ++-
 1 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5b093ce..daa65c1 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -125,19 +125,6 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, 
uint32_t function, int reg)
 
 #endif
 
-static void kvm_trim_features(uint32_t *features, uint32_t supported)
-{
-int i;
-uint32_t mask;
-
-for (i = 0; i  32; ++i) {
-mask = 1U  i;
-if ((*features  mask)  !(supported  mask)) {
-*features = ~mask;
-}
-}
-}
-
 #ifdef CONFIG_KVM_PARA
 struct kvm_para_features {
 int cap;
@@ -186,18 +173,16 @@ int kvm_arch_init_vcpu(CPUState *env)
 
 env-mp_state = KVM_MP_STATE_RUNNABLE;
 
-kvm_trim_features(env-cpuid_features,
-kvm_arch_get_supported_cpuid(env, 1, R_EDX));
+env-cpuid_features = kvm_arch_get_supported_cpuid(env, 1, R_EDX);
 
 i = env-cpuid_ext_features  CPUID_EXT_HYPERVISOR;
-kvm_trim_features(env-cpuid_ext_features,
-kvm_arch_get_supported_cpuid(env, 1, R_ECX));
+env-cpuid_ext_features = kvm_arch_get_supported_cpuid(env, 1, R_ECX);
 env-cpuid_ext_features |= i;
 
-kvm_trim_features(env-cpuid_ext2_features,
-kvm_arch_get_supported_cpuid(env, 0x8001, R_EDX));
-kvm_trim_features(env-cpuid_ext3_features,
-kvm_arch_get_supported_cpuid(env, 0x8001, R_ECX));
+env-cpuid_ext2_features = kvm_arch_get_supported_cpuid(env, 0x8001,
+ R_EDX);
+env-cpuid_ext3_features = kvm_arch_get_supported_cpuid(env, 0x8001,
+ R_ECX);
 
 cpuid_i = 0;
 
-- 
1.6.4

[Qemu-devel] [PATCH 06/13] cpuid: list all known x86 CPUID feature flags

[Andre Przywara]

-cpu ? currently gives us a list of known CPU models. Add host if
using KVM and a list of known CPUID feature flags to the output.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   22 +-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 19d58e1..3f56c50 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -573,10 +573,30 @@ error:
 
 void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
 {
-unsigned int i;
+unsigned int i, j;
+const char **stringlist[] = {feature_name, ext_feature_name,
+ ext2_feature_name, ext3_feature_name};
 
 for (i = 0; i  ARRAY_SIZE(x86_defs); i++)
 (*cpu_fprintf)(f, x86 %16s\n, x86_defs[i].name);
+if (kvm_enabled()) {
+(*cpu_fprintf)(f, x86 %16s\n, host);
+}
+
+(*cpu_fprintf)(f, x86 recognized feature flags:\n);
+for (j = 0; j  4; j++) {
+for (i = 0; i  32; i++) {
+if (j == 2  ((1  i)  EXT2_FEATURE_MASK))
+continue;
+if (stringlist[j][i] == NULL)
+continue;
+(*cpu_fprintf)(f, %s , stringlist[j][i]);
+if (i == 15)
+(*cpu_fprintf)(f, \n);
+}
+(*cpu_fprintf)(f, \n);
+}
+return;
 }
 
 int cpu_x86_register (CPUX86State *env, const char *cpu_model)
-- 
1.6.4

[Qemu-devel] [PATCH 12/13] cpuid: Add kvm32 CPU model

[Andre Przywara]

Create a kvm32 CPU model that describes a least common denominator
for KVM capable guest CPUs. Useful for migration purposes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b03a363..65dcb23 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -263,6 +263,20 @@ static x86_def_t x86_defs[] = {
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
+.name = kvm32,
+.level = 5,
+.family = 15,
+.model = 6,
+.stepping = 1,
+.features = PPRO_FEATURES |
+CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
+.ext_features = CPUID_EXT_SSE3,
+.ext2_features = PPRO_FEATURES  EXT2_FEATURE_MASK,
+.ext3_features = 0,
+.xlevel = 0x8008,
+.model_id = Common 32-bit KVM processor
+},
+{
 .name = coreduo,
 .level = 10,
 .family = 6,
-- 
1.6.4

[Qemu-devel] [PATCH 13/13] cpuid: fix CPUID levels

[Andre Przywara]

Bump up the xlevel number for qemu32 to allow parsing of the processor
name string for this model.
Similiarly the 486 processor should have at least the feature bit
leaf enabled.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 65dcb23..725efe3 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -259,7 +259,7 @@ static x86_def_t x86_defs[] = {
 .stepping = 3,
 .features = PPRO_FEATURES,
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
-.xlevel = 0,
+.xlevel = 0x8004,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
@@ -297,7 +297,7 @@ static x86_def_t x86_defs[] = {
 },
 {
 .name = 486,
-.level = 0,
+.level = 1,
 .family = 4,
 .model = 0,
 .stepping = 0,
-- 
1.6.4

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

[Andre Przywara]

john cooper wrote:


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

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

 ...
John,

first I would like to apologize for sending out my patch series although 
I know that it heavily conflicts with yours. Actually you beat me just 
by hours with yours, I had mine ready on Friday evening and just delayed 
the sending until Monday ;-)


Can you split up the patch into a series of smaller ones (maybe git add 
-i can help you here?). This version is a bit large for proper review 
and mixes fixes and feature additions. Additionally this would help to 
merge our both versions.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[Qemu-devel] [PATCH v3] TCG x86: implement lzcnt emulation

[Andre Przywara]

lzcnt is a AMD Phenom/Barcelona added instruction returning the
number of leading zero bits in a word.
As this is similar to the bsr instruction, reuse the existing
code. There need to be some more changes, though, as lzcnt always
returns a valid value (in opposite to bsr, which has a special
case when the operand is 0).
lzcnt is guarded by the ABM CPUID bit (Fn8000_0001:ECX_5).

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/helper.h|1 +
 target-i386/op_helper.c |   14 --
 target-i386/translate.c |   37 +
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/target-i386/helper.h b/target-i386/helper.h
index ca953f4..6b518ad 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -193,6 +193,7 @@ DEF_HELPER_2(fxsave, void, tl, int)
 DEF_HELPER_2(fxrstor, void, tl, int)
 DEF_HELPER_1(bsf, tl, tl)
 DEF_HELPER_1(bsr, tl, tl)
+DEF_HELPER_2(lzcnt, tl, tl, int)
 
 /* MMX/SSE */
 
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 26fe612..5eea322 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -5479,11 +5479,14 @@ target_ulong helper_bsf(target_ulong t0)
 return count;
 }
 
-target_ulong helper_bsr(target_ulong t0)
+target_ulong helper_lzcnt(target_ulong t0, int wordsize)
 {
 int count;
 target_ulong res, mask;
-
+
+if (wordsize  0  t0 == 0) {
+return wordsize;
+}
 res = t0;
 count = TARGET_LONG_BITS - 1;
 mask = (target_ulong)1  (TARGET_LONG_BITS - 1);
@@ -5491,9 +5494,16 @@ target_ulong helper_bsr(target_ulong t0)
 count--;
 res = 1;
 }
+if (wordsize  0) {
+return wordsize - 1 - count;
+}
 return count;
 }
 
+target_ulong helper_bsr(target_ulong t0)
+{
+   return helper_lzcnt(t0, 0);
+}
 
 static int compute_all_eflags(void)
 {
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 2511943..64bc0a3 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6573,23 +6573,36 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 ot = dflag + OT_WORD;
 modrm = ldub_code(s-pc++);
 reg = ((modrm  3)  7) | rex_r;
-gen_ldst_modrm(s, modrm, ot, OR_TMP0, 0);
+gen_ldst_modrm(s,modrm, ot, OR_TMP0, 0);
 gen_extu(ot, cpu_T[0]);
-label1 = gen_new_label();
-tcg_gen_movi_tl(cpu_cc_dst, 0);
 t0 = tcg_temp_local_new();
 tcg_gen_mov_tl(t0, cpu_T[0]);
-tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, label1);
-if (b  1) {
-gen_helper_bsr(cpu_T[0], t0);
+if ((b  1)  (prefixes  PREFIX_REPZ) 
+(s-cpuid_ext3_features  CPUID_EXT3_ABM)) {
+switch(ot) {
+case OT_WORD: gen_helper_lzcnt(cpu_T[0], t0,
+tcg_const_i32(16)); break;
+case OT_LONG: gen_helper_lzcnt(cpu_T[0], t0,
+tcg_const_i32(32)); break;
+case OT_QUAD: gen_helper_lzcnt(cpu_T[0], t0,
+tcg_const_i32(64)); break;
+}
+gen_op_mov_reg_T0(ot, reg);
 } else {
-gen_helper_bsf(cpu_T[0], t0);
+label1 = gen_new_label();
+tcg_gen_movi_tl(cpu_cc_dst, 0);
+tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, label1);
+if (b  1) {
+gen_helper_bsr(cpu_T[0], t0);
+} else {
+gen_helper_bsf(cpu_T[0], t0);
+}
+gen_op_mov_reg_T0(ot, reg);
+tcg_gen_movi_tl(cpu_cc_dst, 1);
+gen_set_label(label1);
+tcg_gen_discard_tl(cpu_cc_src);
+s-cc_op = CC_OP_LOGICB + ot;
 }
-gen_op_mov_reg_T0(ot, reg);
-tcg_gen_movi_tl(cpu_cc_dst, 1);
-gen_set_label(label1);
-tcg_gen_discard_tl(cpu_cc_src);
-s-cc_op = CC_OP_LOGICB + ot;
 tcg_temp_free(t0);
 }
 break;
-- 
1.6.4

[Qemu-devel] [FOR 0.12][PATCH] cpuid: Fix multicore setup on Intel

[Andre Przywara]

The multicore CPUID code detects whether the guest is an Intel or an
AMD CPU, because the Linux kernel is picky about the CmpLegacy bit.
KVM by default passes through the host's vendor, which was not
catched by the code. So fork out the vendor determining bits into a
separate function to be used from both places and always get the real
vendor.
This fixes KVM's multicore setup on Intel CPUs.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
Reported-by: Dietmar Maurer diet...@proxmox.com
---
 target-i386/helper.c |   46 +++---
 1 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 957b3fc..08e6d6c 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1638,6 +1638,24 @@ static void host_cpuid(uint32_t function, uint32_t count,
 #endif
 }
 
+static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
+ uint32_t *ecx, uint32_t *edx)
+{
+*ebx = env-cpuid_vendor1;
+*edx = env-cpuid_vendor2;
+*ecx = env-cpuid_vendor3;
+
+/* sysenter isn't supported on 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 if you want to use KVM's sysenter/syscall emulation
+ * in compatibility mode and when doing cross vendor migration
+ */
+if (kvm_enabled()  env-cpuid_vendor_override) {
+host_cpuid(0, 0, NULL, ebx, ecx, edx);
+}
+}
+
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
@@ -1654,16 +1672,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 switch(index) {
 case 0:
 *eax = env-cpuid_level;
-*ebx = env-cpuid_vendor1;
-*edx = env-cpuid_vendor2;
-*ecx = env-cpuid_vendor3;
-
-/* sysenter isn't supported on compatibility mode on AMD.  and syscall
- * isn't supported in compatibility mode on Intel.  so advertise the
- * actuall cpu, and say goodbye to migration between different vendors
- * is you use compatibility mode. */
-if (kvm_enabled()  !env-cpuid_vendor_override)
-host_cpuid(0, 0, NULL, ebx, ecx, edx);
+get_cpuid_vendor(env, ebx, ecx, edx);
 break;
 case 1:
 *eax = env-cpuid_version;
@@ -1759,11 +1768,18 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ecx = env-cpuid_ext3_features;
 *edx = env-cpuid_ext2_features;
 
-if (env-nr_cores * env-nr_threads  1 
-env-cpuid_vendor1 == CPUID_VENDOR_AMD_1 
-env-cpuid_vendor2 == CPUID_VENDOR_AMD_2 
-env-cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
-*ecx |= 1  1;/* CmpLegacy bit */
+/* The Linux kernel checks for the CMPLegacy bit and
+ * discards multiple thread information if it is set.
+ * So dont set it here for Intel to make Linux guests happy.
+ */
+if (env-nr_cores * env-nr_threads  1) {
+uint32_t tebx, tecx, tedx;
+get_cpuid_vendor(env, tebx, tecx, tedx);
+if (tebx != CPUID_VENDOR_INTEL_1 ||
+tedx != CPUID_VENDOR_INTEL_2 ||
+tecx != CPUID_VENDOR_INTEL_3) {
+*ecx |= 1  1;/* CmpLegacy bit */
+}
 }
 
 if (kvm_enabled()) {
-- 
1.6.4

[Qemu-devel] [FOR 0.12][PATCH] cpuid: Update QEMU/TCG feature set

[Andre Przywara]

The CPUID features QEMU presented to the guest were not up-to-date
with QEMU's emulated feature set.
Add the missing bits of recent (and not so recent) additions to
QEMU's emulation engine.
For stability reasons only the user mode usable bits are exposed for
now, features like Monitor or CR8LEG are left out.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/helper.c |   25 -
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index f0ecd50..f1ff577 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -139,10 +139,11 @@ static x86_def_t x86_defs[] = {
 CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
 /* this feature is needed for Solaris and isn't fully implemented */
 CPUID_PSE36,
-.ext_features = CPUID_EXT_SSE3,
+.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
 .ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-.ext3_features = CPUID_EXT3_SVM,
+.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
+CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
 .xlevel = 0x800A,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
@@ -159,18 +160,19 @@ static x86_def_t x86_defs[] = {
 .features = PPRO_FEATURES | 
 CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
 CPUID_PSE36,
-/* Missing: CPUID_EXT_CX16, CPUID_EXT_POPCNT */
-.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR,
+.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
+CPUID_EXT_POPCNT,
 /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
 .ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
 CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
 CPUID_EXT2_FFXSR,
-/* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
-CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
+/* Missing: CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
+CPUID_EXT3_CR8LEG,
 CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
 CPUID_EXT3_OSVW, CPUID_EXT3_IBS */
-.ext3_features = CPUID_EXT3_SVM,
+.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
+CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
 .xlevel = 0x801A,
 .model_id = AMD Phenom(tm) 9550 Quad-Core Processor
 },
@@ -191,7 +193,7 @@ static x86_def_t x86_defs[] = {
CPUID_EXT_TM2, CPUID_EXT_CX16, CPUID_EXT_XTPR, CPUID_EXT_PDCM */
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3,
 .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-/* Missing: .ext3_features = CPUID_EXT3_LAHF_LM */
+.ext3_features = CPUID_EXT3_LAHF_LM,
 .xlevel = 0x8008,
 .model_id = Intel(R) Core(TM)2 Duo CPU T7700  @ 2.40GHz,
 },
@@ -229,7 +231,7 @@ static x86_def_t x86_defs[] = {
 .model = 3,
 .stepping = 3,
 .features = PPRO_FEATURES,
-.ext_features = CPUID_EXT_SSE3,
+.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
 .xlevel = 0,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
@@ -1791,11 +1793,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 }
 
 if (kvm_enabled()) {
-/* Nested SVM not yet supported in KVM */
+/* Nested SVM not yet supported in upstream QEMU */
 *ecx = ~CPUID_EXT3_SVM;
-} else {
-/* AMD 3DNow! is not supported in QEMU */
-*edx = ~(CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT);
 }
 break;
 case 0x8002:
-- 
1.6.4

[Qemu-devel] [PATCH] configure: fix --sysconfdir specification

[Andre Przywara]

--sysconfdir requires a parameter (the path), this should be reflected
in the case pattern.

Reported-by: Frank Arnold frank.arn...@amd.com
Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 configure |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index f55baf0..d3b4df9 100755
--- a/configure
+++ b/configure
@@ -495,7 +495,7 @@ for opt do
 static=yes
 LDFLAGS=-static $LDFLAGS
   ;;
-  --sysconfdir) sysconfdir=$optarg
+  --sysconfdir=*) sysconfdir=$optarg
   ;;
   --disable-sdl) sdl=no
   ;;
-- 
1.6.1.3

[Qemu-devel] [PATCH] install: honor DESTDIR on sysconfdir population

[Andre Przywara]

When creating and populating $sysconfdir, we should prepend $DESTDIR
as we do with all other paths.

Reported-by: Frank Arnold frank.arn...@amd.com
Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 Makefile |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index bf2cef2..2066c12 100644
--- a/Makefile
+++ b/Makefile
@@ -194,8 +194,8 @@ ifdef CONFIG_POSIX
 endif
 
 install-sysconfig:
-   $(INSTALL_DIR) $(sysconfdir)/qemu
-   $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf 
$(sysconfdir)/qemu
+   $(INSTALL_DIR) $(DESTDIR)$(sysconfdir)/qemu
+   $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf 
$(DESTDIR)$(sysconfdir)/qemu
 
 install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
$(INSTALL_DIR) $(DESTDIR)$(bindir)
-- 
1.6.4

[Qemu-devel] [PATCH 03/13] x86/cpuid: fix missing feature set bits

[Andre Przywara]

This one was accidently removed with commit
bb0300dc57c10b3721451b0ff566a03f9276cc77

Signed-off-by: Andre Przywara andre.przyw...@amd.com

---
 target-i386/cpuid.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 5e4f057..1bceb4b 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -754,6 +754,7 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-pat = 0x0007040600070406ULL;
 env-cpuid_ext_features = def-ext_features;
 env-cpuid_ext2_features = def-ext2_features;
+env-cpuid_ext3_features = def-ext3_features;
 env-cpuid_xlevel = def-xlevel;
 env-cpuid_kvm_features = def-kvm_features;
 {
-- 
1.6.4

[Qemu-devel] [PATCH 00/13] x86/cpuid: cleanups and fixes

[Andre Przywara]

Hi,

a rebased and refined version of my CPUID cleanup series. This should
now apply on top of git head. Compared to the last post this leaves
out the bits already done by John Cooper's patch. Also I added a separate
patch to deal with an accidently removed line (patch 3/13).

This patchset cleans up the CPUID handling code in QEMU. The biggest change
is obviously the move of the CPUID function to a separate file (cpuid.c).
This helps to split up a rather large source file, which's name (helper.c) is
also a bit misleading.
Additionally the rest of the patches beautifies or simplifies some code.
Feature additions are:
 5/13: add missing CPUID feature bit names (updated)
 8/13: -cpu host propagates more CPUID leafs, so that the cache topology
   will be visibile in the guest
 9/13: add CPUID feature bit trimming for TCG: Features not supported by
   the emulator will be masked out.
10/13: always show all CPU types: also expose the newer (64bit) CPU types
   for the i386 emulator. 64bit features will be masked out due to 9/13.
11/13: add kvm32 CPU model: Per popular request add a counterpart to kvm64
   describing a basic hardware virtualization capable CPU for migration
   purposes.
13/13: Set the feature bits of qemu{32,64} to the set supported by TCG.

The last patch is actually debatable, as it changes the default CPUID bits
seen by guest. On the other hand it is only consequent to do so, as
qemu{32,64} are meant to be artifical CPUs.

More details in the commit messages.

Please review, comment and apply.

Regards,
Andre.

[Qemu-devel] [PATCH 06/13] x86/cpuid: add host to the list of supported CPU models

[Andre Przywara]

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 56ae71a..1e1c25b 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -757,6 +757,9 @@ void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, 
const char *fmt, ...),
 (*cpu_fprintf)(f, \n);
 }
 }
+if (kvm_enabled()) {
+(*cpu_fprintf)(f, x86 %16s\n, [host]);
+}
 }
 
 int cpu_x86_register (CPUX86State *env, const char *cpu_model)
-- 
1.6.4

[Qemu-devel] [PATCH 08/13] x86/cpuid: propagate further CPUID leafs when -cpu host

[Andre Przywara]

-cpu host currently only propagates the CPU's family/model/stepping,
the brand name and the feature bits.
Add a whitelist of safe CPUID leafs to let the guest see the actual
CPU's cache details and other things.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpu.h   |6 +-
 target-i386/cpuid.c |   32 
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c540213..72addd7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -581,6 +581,10 @@ typedef struct {
 
 #define NB_MMU_MODES 2
 
+#define CPUID_FLAGS_BUILTIN  (1  0)
+#define CPUID_FLAGS_VENDOR_OVERRIDE  (1  1)
+#define CPUID_FLAGS_HOST (1  2) 
+
 typedef struct CPUX86State {
 /* standard registers */
 target_ulong regs[CPU_NB_REGS];
@@ -685,7 +689,7 @@ typedef struct CPUX86State {
 uint32_t cpuid_ext2_features;
 uint32_t cpuid_ext3_features;
 uint32_t cpuid_apic_id;
-int cpuid_vendor_override;
+uint32_t cpuid_flags;
 
 /* MTRRs */
 uint64_t mtrr_fixed[11];
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 1e1c25b..14a10e9 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -213,7 +213,6 @@ typedef struct x86_def_t {
 uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
 uint32_t xlevel;
 char model_id[48];
-int vendor_override;
 uint32_t flags;
 } x86_def_t;
 
@@ -481,7 +480,7 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 x86_cpu_def-ext2_features = edx;
 x86_cpu_def-ext3_features = ecx;
 cpu_x86_fill_model_id(x86_cpu_def-model_id);
-x86_cpu_def-vendor_override = 0;
+x86_cpu_def-flags = CPUID_FLAGS_HOST;
 
 return 0;
 }
@@ -625,7 +624,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 x86_cpu_def-vendor2 |= ((uint8_t)val[i + 4])  (8 * i);
 x86_cpu_def-vendor3 |= ((uint8_t)val[i + 8])  (8 * i);
 }
-x86_cpu_def-vendor_override = 1;
+x86_cpu_def-flags |= CPUID_FLAGS_VENDOR_OVERRIDE;
 } else if (!strcmp(featurestr, model_id)) {
 pstrcpy(x86_cpu_def-model_id, sizeof(x86_cpu_def-model_id),
 val);
@@ -723,7 +722,8 @@ void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, 
const char *fmt, ...),
 return;
 }
 for (def = x86_defs; def; def = def-next) {
-snprintf(buf, sizeof (buf), def-flags ? [%s]: %s, def-name);
+snprintf(buf, sizeof (buf),
+ def-flags  CPUID_FLAGS_BUILTIN ? [%s]: %s, def-name);
 if (model || dump) {
 (*cpu_fprintf)(f, x86 %16s  %-48s\n, buf, def-model_id);
 } else {
@@ -777,7 +777,7 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-cpuid_vendor2 = CPUID_VENDOR_INTEL_2;
 env-cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
 }
-env-cpuid_vendor_override = def-vendor_override;
+env-cpuid_flags = def-flags;
 env-cpuid_level = def-level;
 if (def-family  0x0f)
 env-cpuid_version = 0xf00 | ((def-family - 0x0f)  20);
@@ -923,7 +923,7 @@ void x86_cpudef_setup(void)
 
 for (i = 0; i  ARRAY_SIZE(builtin_x86_defs); ++i) {
 builtin_x86_defs[i].next = x86_defs;
-builtin_x86_defs[i].flags = 1;
+builtin_x86_defs[i].flags |= CPUID_FLAGS_BUILTIN;
 x86_defs = builtin_x86_defs[i];
 }
 #if !defined(CONFIG_USER_ONLY)
@@ -944,22 +944,38 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t 
*ebx,
  * this if you want to use KVM's sysenter/syscall emulation
  * in compatibility mode and when doing cross vendor migration
  */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled()  env-cpuid_flags  CPUID_FLAGS_VENDOR_OVERRIDE) {
 host_cpuid(0, 0, NULL, ebx, ecx, edx);
 }
 }
 
+/* safe CPUID leafs to propagate to guest if -cpu host is specified */
+#define CPUID_LEAF_PROPAGATE ((1  0x02) | (1  0x04) | (1  0x05) |\
+  (1  0x0D))
+#define CPUID_LEAF_PROPAGATE_EXTENDED ((1  0x05) | (1  0x06) |\
+   (1  0x08) | (1  0x19) | (1  0x1A))
+
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
 {
-/* test if maximum index reached */
 if (index  0x8000) {
+/* test if maximum index reached */
 if (index  env-cpuid_xlevel)
 index = env-cpuid_level;
+if ((env-cpuid_flags  CPUID_FLAGS_HOST) 
+((1  (index - 0x8000))  CPUID_LEAF_PROPAGATE_EXTENDED)) {
+host_cpuid(index, count, eax, ebx, ecx, edx);
+return;
+}
 } else {
 if (index  env-cpuid_level)
 index = env-cpuid_level;
+if ((env-cpuid_flags  CPUID_FLAGS_HOST

[Qemu-devel] [PATCH 12/13] x86/cpuid: fix CPUID levels

[Andre Przywara]

Bump up the xlevel number for qemu32 to allow parsing of the processor
name string for this model.
Similiarly the 486 processor should have at least the feature bit
leaf enabled.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index e3c36be..076f6cc 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -356,7 +356,7 @@ static x86_def_t builtin_x86_defs[] = {
 .stepping = 3,
 .features = PPRO_FEATURES,
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
-.xlevel = 0,
+.xlevel = 0x8004,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
@@ -394,7 +394,7 @@ static x86_def_t builtin_x86_defs[] = {
 },
 {
 .name = 486,
-.level = 0,
+.level = 1,
 .family = 4,
 .model = 0,
 .stepping = 0,
-- 
1.6.4

[Qemu-devel] [PATCH 09/13] x86/cpuid: add TCG feature bit trimming

[Andre Przywara]

In KVM we trim the user provided CPUID bits to match the host CPU's
one. Introduce a similar feature to QEMU/TCG. Create a mask of TCG's
capabilities and apply it to the user bits.
This allows to let the CPU models reflect their native archetypes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 14a10e9..a6b2362 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -229,6 +229,20 @@ typedef struct x86_def_t {
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
 #define EXT2_FEATURE_MASK 0x0183F3FF
 
+#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
+  CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
+  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+  CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
+  CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
+#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | \
+  CPUID_EXT_CX16 | CPUID_EXT_POPCNT | CPUID_EXT_XSAVE | \
+  CPUID_EXT_HYPERVISOR)
+#define TCG_EXT2_FEATURES ((TCG_FEATURES  EXT2_FEATURE_MASK) | \
+  CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
+  CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT)
+#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
+  CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+
 /* maintains list of cpu model definitions
  */
 static x86_def_t *x86_defs = {NULL};
@@ -792,6 +806,16 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env-cpuid_ext3_features = def-ext3_features;
 env-cpuid_xlevel = def-xlevel;
 env-cpuid_kvm_features = def-kvm_features;
+if (!kvm_enabled()) {
+env-cpuid_features = TCG_FEATURES;
+env-cpuid_ext_features = TCG_EXT_FEATURES;
+env-cpuid_ext2_features = (TCG_EXT2_FEATURES
+#ifdef TARGET_X86_64
+| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
+#endif
+);
+env-cpuid_ext3_features = TCG_EXT3_FEATURES;
+}
 {
 const char *model_id = def-model_id;
 int c, len, i;
-- 
1.6.4

[Qemu-devel] [PATCH 04/13] x86/cpuid: moved host_cpuid function and remove prototype

[Andre Przywara]

the host_cpuid function was located at the end of the file and had
a prototype before it's first use. Move it up and remove the
prototype.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   71 --
 1 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 1bceb4b..fa36942 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -92,8 +92,40 @@ typedef struct model_features_t {
 int check_cpuid = 0;
 int enforce_cpuid = 0;
 
-static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
-   uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+static void host_cpuid(uint32_t function, uint32_t count,
+   uint32_t *eax, uint32_t *ebx,
+   uint32_t *ecx, uint32_t *edx)
+{
+#if defined(CONFIG_KVM)
+uint32_t vec[4];
+
+#ifdef __x86_64__
+asm volatile(cpuid
+ : =a(vec[0]), =b(vec[1]),
+   =c(vec[2]), =d(vec[3])
+ : 0(function), c(count) : cc);
+#else
+asm volatile(pusha \n\t
+ cpuid \n\t
+ mov %%eax, 0(%2) \n\t
+ mov %%ebx, 4(%2) \n\t
+ mov %%ecx, 8(%2) \n\t
+ mov %%edx, 12(%2) \n\t
+ popa
+ : : a(function), c(count), S(vec)
+ : memory, cc);
+#endif
+
+if (eax)
+   *eax = vec[0];
+if (ebx)
+   *ebx = vec[1];
+if (ecx)
+   *ecx = vec[2];
+if (edx)
+   *edx = vec[3];
+#endif
+}
 
 #define iswhite(c) ((c)  ((c) = ' ' || '~'  (c)))
 
@@ -896,41 +928,6 @@ void x86_cpudef_setup(void)
 #endif
 }
 
-static void host_cpuid(uint32_t function, uint32_t count,
-   uint32_t *eax, uint32_t *ebx,
-   uint32_t *ecx, uint32_t *edx)
-{
-#if defined(CONFIG_KVM)
-uint32_t vec[4];
-
-#ifdef __x86_64__
-asm volatile(cpuid
- : =a(vec[0]), =b(vec[1]),
-   =c(vec[2]), =d(vec[3])
- : 0(function), c(count) : cc);
-#else
-asm volatile(pusha \n\t
- cpuid \n\t
- mov %%eax, 0(%2) \n\t
- mov %%ebx, 4(%2) \n\t
- mov %%ecx, 8(%2) \n\t
- mov %%edx, 12(%2) \n\t
- popa
- : : a(function), c(count), S(vec)
- : memory, cc);
-#endif
-
-if (eax)
-   *eax = vec[0];
-if (ebx)
-   *ebx = vec[1];
-if (ecx)
-   *ecx = vec[2];
-if (edx)
-   *edx = vec[3];
-#endif
-}
-
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
  uint32_t *ecx, uint32_t *edx)
 {
-- 
1.6.4

[Qemu-devel] [PATCH 13/13] x86/cpuid: Update qemu64/32 CPU models

[Andre Przywara]

Since we now have a real TCG feature set, use it to describe the
artificial qemu CPUs (both 64 and 32-bit). If new features are added
to TCG, the capability of qemu64/32 will automatically be adjusted.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   25 +
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 076f6cc..ec43596 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -227,6 +227,7 @@ typedef struct x86_def_t {
   CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
   CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
+#define EXT2_FEATURES_64 (CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX)
 #define EXT2_FEATURE_MASK 0x0183F3FF
 
 #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
@@ -259,16 +260,12 @@ static x86_def_t builtin_x86_defs[] = {
 .family = 6,
 .model = 2,
 .stepping = 3,
-.features = PPRO_FEATURES | 
-/* these features are needed for Win64 and aren't fully implemented */
-CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
-/* this feature is needed for Solaris and isn't fully implemented */
-CPUID_PSE36,
-.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
-.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
-CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
-CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
+.features = TCG_FEATURES,
+.ext_features = TCG_EXT_FEATURES,
+/* 3DNow! is deprecated, so leave it out of the default feature set */
+.ext2_features = (TCG_EXT2_FEATURES | EXT2_FEATURES_64) 
+~(CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT),
+.ext3_features = TCG_EXT3_FEATURES,
 .xlevel = 0x800A,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
@@ -354,8 +351,12 @@ static x86_def_t builtin_x86_defs[] = {
 .family = 6,
 .model = 3,
 .stepping = 3,
-.features = PPRO_FEATURES,
-.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
+.features = TCG_FEATURES,
+.ext_features = TCG_EXT_FEATURES,
+/* 3DNow! is deprecated, so leave it out of the default feature set */
+.ext2_features = TCG_EXT2_FEATURES 
+   ~(CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT),
+.ext3_features = TCG_EXT3_FEATURES,
 .xlevel = 0x8004,
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
-- 
1.6.4

[Qemu-devel] [PATCH 02/13] x86/cpuid: replace magic number with named constant

[Andre Przywara]

CPUID leaf Fn8000_0001.EDX contains a copy of many Fn_0001.EDX bits.
Define a name for this mask to improve readability and avoid typos.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index ca1aedd..5e4f057 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -196,6 +196,7 @@ typedef struct x86_def_t {
   CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
   CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
+#define EXT2_FEATURE_MASK 0x0183F3FF
 
 /* maintains list of cpu model definitions
  */
@@ -220,7 +221,7 @@ static x86_def_t builtin_x86_defs[] = {
 /* this feature is needed for Solaris and isn't fully implemented */
 CPUID_PSE36,
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
 .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
 CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
@@ -243,7 +244,7 @@ static x86_def_t builtin_x86_defs[] = {
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
 CPUID_EXT_POPCNT,
 /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | 
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
 CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
 CPUID_EXT2_FFXSR,
@@ -293,7 +294,7 @@ static x86_def_t builtin_x86_defs[] = {
 /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16,
 /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) |
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) |
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
 /* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
 CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
@@ -380,7 +381,7 @@ static x86_def_t builtin_x86_defs[] = {
 .model = 2,
 .stepping = 3,
 .features = PPRO_FEATURES | CPUID_PSE36 | CPUID_VME | CPUID_MTRR | 
CPUID_MCA,
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | CPUID_EXT2_MMXEXT | 
CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | 
CPUID_EXT2_MMXEXT | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
 .xlevel = 0x8008,
 /* XXX: put another string ? */
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
@@ -401,7 +402,7 @@ static x86_def_t builtin_x86_defs[] = {
 CPUID_EXT_SSE3 /* PNI */ | CPUID_EXT_SSSE3,
 /* Missing: CPUID_EXT_DSCPL | CPUID_EXT_EST |
  * CPUID_EXT_TM2 | CPUID_EXT_XTPR */
-.ext2_features = (PPRO_FEATURES  0x0183F3FF) | CPUID_EXT2_NX,
+.ext2_features = (PPRO_FEATURES  EXT2_FEATURE_MASK) | CPUID_EXT2_NX,
 /* Missing: .ext3_features = CPUID_EXT3_LAHF_LM */
 .xlevel = 0x800A,
 .model_id = Intel(R) Atom(TM) CPU N270   @ 1.60GHz,
-- 
1.6.4

[Qemu-devel] [PATCH 07/13] x86/cpuid: remove unnecessary kvm_trim function

[Andre Przywara]

Correct me if I am wrong, but kvm_trim looks like a really bloated
implementation of a bitwise AND. So remove this function and replace
it with the real stuff(TM).

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/kvm.c |   27 ++-
 1 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 40f8303..f77e488 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -133,19 +133,6 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, 
uint32_t function, int reg)
 
 #endif
 
-static void kvm_trim_features(uint32_t *features, uint32_t supported)
-{
-int i;
-uint32_t mask;
-
-for (i = 0; i  32; ++i) {
-mask = 1U  i;
-if ((*features  mask)  !(supported  mask)) {
-*features = ~mask;
-}
-}
-}
-
 #ifdef CONFIG_KVM_PARA
 struct kvm_para_features {
 int cap;
@@ -191,18 +178,16 @@ int kvm_arch_init_vcpu(CPUState *env)
 
 env-mp_state = KVM_MP_STATE_RUNNABLE;
 
-kvm_trim_features(env-cpuid_features,
-kvm_arch_get_supported_cpuid(env, 1, R_EDX));
+env-cpuid_features = kvm_arch_get_supported_cpuid(env, 1, R_EDX);
 
 i = env-cpuid_ext_features  CPUID_EXT_HYPERVISOR;
-kvm_trim_features(env-cpuid_ext_features,
-kvm_arch_get_supported_cpuid(env, 1, R_ECX));
+env-cpuid_ext_features = kvm_arch_get_supported_cpuid(env, 1, R_ECX);
 env-cpuid_ext_features |= i;
 
-kvm_trim_features(env-cpuid_ext2_features,
-kvm_arch_get_supported_cpuid(env, 0x8001, R_EDX));
-kvm_trim_features(env-cpuid_ext3_features,
-kvm_arch_get_supported_cpuid(env, 0x8001, R_ECX));
+env-cpuid_ext2_features = kvm_arch_get_supported_cpuid(env, 0x8001,
+ R_EDX);
+env-cpuid_ext3_features = kvm_arch_get_supported_cpuid(env, 0x8001,
+ R_ECX);
 
 cpuid_i = 0;
 
-- 
1.6.4

[Qemu-devel] [PATCH 05/13] x86/cpuid: add missing CPUID feature flag names

[Andre Przywara]

Some CPUID feature flags had no string value, so they could not be
switched on or off from the command line.
Add names for the missing ones mentioned in the current public CPUID
specification from both Intel and AMD. Those only mentioned in the
Linux kernel source I put as comments.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index fa36942..56ae71a 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -42,14 +42,14 @@ static const char *feature_name[] = {
 ht /* Intel htt */, tm, ia64, pbe,
 };
 static const char *ext_feature_name[] = {
-pni|sse3 /* Intel,AMD sse3 */, NULL, NULL, monitor,
-ds_cpl, vmx, NULL /* Linux smx */, est,
+pni|sse3 /* Intel,AMD sse3 */, pclmuldq, dtes64, monitor,
+ds_cpl, vmx, smx, est,
 tm2, ssse3, cid, NULL,
-NULL, cx16, xtpr, NULL,
+fma, cx16, xtpr, pdcm,
 NULL, NULL, dca, sse4.1|sse4_1,
-sse4.2|sse4_2, x2apic, NULL, popcnt,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, hypervisor,
+sse4.2|sse4_2, x2apic, movbe, popcnt,
+NULL, aes, xsave, osxsave,
+avx, NULL, NULL, hypervisor,
 };
 static const char *ext2_feature_name[] = {
 fpu, vme, de, pse,
@@ -64,9 +64,9 @@ static const char *ext2_feature_name[] = {
 static const char *ext3_feature_name[] = {
 lahf_lm /* AMD LahfSahf */, cmp_legacy, svm, extapic /* AMD 
ExtApicSpace */,
 cr8legacy /* AMD AltMovCr8 */, abm, sse4a, misalignsse,
-3dnowprefetch, osvw, NULL /* Linux ibs */, NULL,
+3dnowprefetch, osvw, ibs, xop,
 skinit, wdt, NULL, NULL,
-NULL, NULL, NULL, NULL,
+fma4, NULL, cvt16, nodeid_msr,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
-- 
1.6.4

[Qemu-devel] [PATCH 11/13] x86/cpuid: Add kvm32 CPU model

[Andre Przywara]

Create a kvm32 CPU model that describes a least common denominator
for KVM capable guest CPUs. Useful for migration purposes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index e7e9228..e3c36be 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -360,6 +360,20 @@ static x86_def_t builtin_x86_defs[] = {
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
+.name = kvm32,
+.level = 5,
+.family = 15,
+.model = 6,
+.stepping = 1,
+.features = PPRO_FEATURES |
+CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
+.ext_features = CPUID_EXT_SSE3,
+.ext2_features = PPRO_FEATURES  EXT2_FEATURE_MASK,
+.ext3_features = 0,
+.xlevel = 0x8008,
+.model_id = Common 32-bit KVM processor
+},
+{
 .name = coreduo,
 .level = 10,
 .family = 6,
-- 
1.6.4

[Qemu-devel] tainted Linux kernel in default SMP QEMU/KVM guests

[Andre Przywara]

Hi,

since the default CPU model for QEMU (qemu64) is an AMD K7, the Linux 
kernel complains and taints the kernel when it detects multiple 
processors. The reason for this is a check for SMP safe CPUs in 
arch/x86/kernel/cpu/amd.c:amd_k7_smp_check(). In recent kernels (since 
about 2.6.29) this in only executed in 32bit kernels, since the check 
has been #ifdef'ed CONFIG_X86_32.
The failing check triggers a nasty dump along with a call trace and 
taints the kernel, this confuses users, so I want to get rid of it.
One can work around it (using -cpu kvm64 or -cpu host), but I want to 
avoid it in the default case, too.


I see these possible solutions:

1) Change the default CPUID bits from 6/2/3 to 6/6/1, this passes the 
Linux kernel check. But I am not sure if that would introduce 
regressions, since some OSes apply quirks if they detect certain models 
(like we had with the sysenter issue in the past)


2) Only change the CPUID bits to 6/6/1 if we use SMP. Still has the 
above drawback, but would be limited to SMP guests only.


3) Set kvm64/kvm32 as the default CPU model if KVM is enabled. This 
would limit the report and taint to TCG, where SMP is rarely used. 
Additionally less people (if any) use it for production systems.


4) Make the Linux' kernel quirk dependent on the missing hypervisor bit. 
I don't think this will be accepted easily upstream (and I don't want to 
support Ingo's recent ideas ;-), also this would not fix older kernels.


I can easily provide patches for all solutions, but I'd like to get 
advice from people on which one to pursue.


Regards,
Andre.

 --
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

[Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override

[Andre Przywara]

Avi Kivity wrote:

On 04/11/2010 10:21 PM, Andre Przywara wrote:

the meaning of vendor_override is actually the opposite of how it
is currently used :-(
Fix it to allow KVM to export the non-native CPUID vendor if
explicitly requested by the user.

Signed-off-by: Andre Przywaraandre.przyw...@amd.com
---
  target-i386/helper.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

I will send a refactoring patch including this fix for git HEAD later.
   


The protocol is to first fix HEAD, then backport, so we don't introduce 
regressions inadvertently.
Well, I just followed the call for 0.12.4. Actually this is the 
backport, one cannot easily cherry-pick the upstream patch, because it 
is in a different file now. I will send the (separated) upstream patch 
in a few minutes. Feel free to apply them in whatever order you prefer ;-)



Regards,
Andre.

P.S. I also have a qemu-kvm patch in the queue (because there was a 
separate workaround for this bug).


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

[Qemu-devel] [PATCH][STABLE] fix CPUID vendor override

[Andre Przywara]

the meaning of vendor_override is actually the opposite of how it
is currently used :-(
Fix it to allow KVM to export the non-native CPUID vendor if
explicitly requested by the user.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

I will send a refactoring patch including this fix for git HEAD later.

Regards,
Andre.

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9d7fec3..c17adc1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1655,7 +1655,7 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t 
*ebx,
  * this if you want to use KVM's sysenter/syscall emulation
  * in compatibility mode and when doing cross vendor migration
  */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled()  ! env-cpuid_vendor_override) {
 host_cpuid(0, 0, NULL, ebx, ecx, edx);
 }
 }
-- 
1.6.4

[Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override

[Andre Przywara]

Avi Kivity wrote:

On 04/11/2010 10:21 PM, Andre Przywara wrote:

the meaning of vendor_override is actually the opposite of how it
is currently used :-(
Fix it to allow KVM to export the non-native CPUID vendor if
explicitly requested by the user.

Signed-off-by: Andre Przywaraandre.przyw...@amd.com
---
  target-i386/helper.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

I will send a refactoring patch including this fix for git HEAD later.

Regards,
Andre.

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9d7fec3..c17adc1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1655,7 +1655,7 @@ static void get_cpuid_vendor(CPUX86State *env, 
uint32_t *ebx,

   * this if you want to use KVM's sysenter/syscall emulation
   * in compatibility mode and when doing cross vendor migration
   */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled()  ! env-cpuid_vendor_override) {
  host_cpuid(0, 0, NULL, ebx, ecx, edx);
  }
  }
   


Why is the original code wrong?  I would say vendor_override means 
overriding the qemu-picked vendor ID in favour of the host cpuid.
I meant it to mean: override the automatically chosen vendor ID (which 
is the host ID in case of KVM). I think the reason for KVM to use the 
host ID is valid, so I wanted to have an explicit override only if the 
user says so.
If you look at the code, you will see that it is initialized to 0 and 
only set to 1 if one specifies an explicit vendor ID on the command line.
Honestly I cannot say how this bug slipped through, I can only guess 
that I tricked myself while making the final version of the patch 
(lost-in-branches(TM))


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

[Qemu-devel] [PATCH] x86/cpuid: Add kvm32 CPU model

[Andre Przywara]

Create a kvm32 CPU model that describes a least common denominator
for KVM capable guest CPUs. Useful for migration purposes.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpuid.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index a80baa4..76b897d 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -363,6 +363,20 @@ static x86_def_t builtin_x86_defs[] = {
 .model_id = QEMU Virtual CPU version  QEMU_VERSION,
 },
 {
+.name = kvm32,
+.level = 5,
+.family = 15,
+.model = 6,
+.stepping = 1,
+.features = PPRO_FEATURES |
+CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
+.ext_features = CPUID_EXT_SSE3,
+.ext2_features = PPRO_FEATURES  EXT2_FEATURE_MASK,
+.ext3_features = 0,
+.xlevel = 0x8008,
+.model_id = Common 32-bit KVM processor
+},
+{
 .name = coreduo,
 .level = 10,
 .family = 6,
-- 
1.6.4

Re: [Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override

[Andre Przywara]

Aurelien Jarno wrote:

On Sun, Apr 11, 2010 at 09:49:40PM +0200, Andre Przywara wrote:

Avi Kivity wrote:

On 04/11/2010 10:21 PM, Andre Przywara wrote:

the meaning of vendor_override is actually the opposite of how it
is currently used :-(
Fix it to allow KVM to export the non-native CPUID vendor if
explicitly requested by the user.

Signed-off-by: Andre Przywaraandre.przyw...@amd.com
---
  target-i386/helper.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

I will send a refactoring patch including this fix for git HEAD later.

Regards,
Andre.

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9d7fec3..c17adc1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1655,7 +1655,7 @@ static void get_cpuid_vendor(CPUX86State *env,  
uint32_t *ebx,

   * this if you want to use KVM's sysenter/syscall emulation
   * in compatibility mode and when doing cross vendor migration
   */
-if (kvm_enabled()  env-cpuid_vendor_override) {
+if (kvm_enabled()  ! env-cpuid_vendor_override) {
  host_cpuid(0, 0, NULL, ebx, ecx, edx);
  }
  }
   
Why is the original code wrong?  I would say vendor_override means  
overriding the qemu-picked vendor ID in favour of the host cpuid.
I meant it to mean: override the automatically chosen vendor ID (which  
is the host ID in case of KVM). I think the reason for KVM to use the  
host ID is valid, so I wanted to have an explicit override only if the  
user says so.
If you look at the code, you will see that it is initialized to 0 and  
only set to 1 if one specifies an explicit vendor ID on the command line.
Honestly I cannot say how this bug slipped through, I can only guess  
that I tricked myself while making the final version of the patch  
(lost-in-branches(TM))




While it clearly fixes the -cpu vendor option when kvm is enabled, it
also forces the vendor id to the one of the host. Try for example -cpu
qemu64. Before your patch the vendor id is Authentic AMD and after
your patch it is Genuine Intel

You are using the wrong CPU ;-)
This is actually correct for KVM. (or are you seeing this with TCG?)
To make sure we are actually talk about the same problem, the intended 
behavior is:

With TCG:
- always inject the configured vendor (either hard-coded, in config 
files or via ,vendor= commandline)

With KVM:
- by default inject the host's vendor
- if the user specifies ,vendor= on the commandline, use this 
instead of the host's vendor

- all pre-configured vendors (hard-coded, config file) are ignored

The reason KVM is injecting the host's vendor is the following:
There is a nasty difference between AMD and Intel chips in supporting 
syscall and sysenter in _compat_ mode (that is: running a 32bit binary 
while the OS is running 64bit). This mode is 99% compatible with the 
legacy protected mode, but AMD does not support sysenter here and Intel 
does not support syscall. So for instance the Linux kernel checks the 
vendor id and tells the userspace to use the supported instruction. On 
TCG we don't care as we properly emulate both(?), but on KVM the 
instruction is called natively. This leads to a crash (#UD exception) if 
the wrong vendor is used. So a long time ago the workaround of always 
propagating the host vendor was introduced.
Later last year I introduced emulation of the missing instructions, so 
to overcome the former limitation (and allow for more hacking with KVM) 
I introduced the vendor override. Sadly my patch was broken (I used a 
slightly different version for development), that's why this fix. 
qemu-kvm had an additional fix to kind of work-around this bug.


Hope that clarifies the issue, if you meant something else, tell me.

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

Re: [Qemu-devel] [PATCH] fix gcc4 compile warnings

[Andre Przywara]

andrzej zaborowski wrote:

On 30/11/2007, Andre Przywara [EMAIL PROTECTED] wrote:

  These casts are not the right way to get rid of the warnings, as are
  some of the casts in other files in qemu_put_* and qemu_get_*
  arguments. In this case the warnings are true positives and the bugs
  causing the warnings have to be addressed instead of just the
  warnings.

Are you sure of that? Most of the fixes are like this:
  -qemu_put_be32s(f, s-count_shift);
  +qemu_put_be32s(f, (uint32_t *)s-count_shift);
qemu_put_be32s is (QEMUFile *f, const uint32_t *pv), but after all the
2nd argument is only a _pointer_ to an unsigned variable, the size is
the same (thanks to the C99 explicit types).


count_shift is an int so it is the machine word size, not necesarily
32-bit AFAIK. Otherwise I guess gcc wouldn't warn. You can possibly
use:

qemu_put_be32(f, s-count_shift);

Or point qemu_put_be32_s() to a int32_t/uint32_t variable.

Oh, you are right, I missed that. Will rework the patch.


What solution do you prefer for the opaque types? I have used the simple:
  -void *args[MAX_ARGS];
  +intptr_t args[MAX_ARGS];
A more portable and clean solution would be this:
-void *args[MAX_ARGS];
+union
+{
+void* ptr;
+int i;
+} args[MAX_ARGS];
If you prefer this, I can change the patch accordingly.


I'm not sure why you get a warning here and I'm unable to run a build
at the moment. A void * should be able to store some (unknown size)
integer regardless of the platform.


sizeof(void*) is 8, whereas sizeof(int) is 4 on a 64bit platform. If I 
assign a 32bit value to a 64bit (pointer) variable, GCC4 says:

warning: cast to pointer from integer of different size

You are right that a pointer _should_ be able to hold an integer, but 
AFAIK this is not guaranteed (aka written in the standard). To avoid 
those problems intptr_t was introduced.
But I think opaque types in C are broken anyway, so the union version 
makes it at least more readable, since you explicitly say integer or 
pointer at the assignment and usage. But I have no problem with using 
intptr_t, the union was just a suggestion.


What about fixing monitor.c#monitor_handle_command in general and avoid 
the opaque types at all?


But wouldn't the union version work well even on win64?


Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x84917

Re: [Qemu-devel] [PATCH] fix gcc4 compile warnings

[Andre Przywara]

Andrzej,

 These casts are not the right way to get rid of the warnings, as are
 some of the casts in other files in qemu_put_* and qemu_get_*
 arguments. In this case the warnings are true positives and the bugs
 causing the warnings have to be addressed instead of just the
 warnings.

Are you sure of that? Most of the fixes are like this:
 -qemu_put_be32s(f, s-count_shift);
 +qemu_put_be32s(f, (uint32_t *)s-count_shift);
qemu_put_be32s is (QEMUFile *f, const uint32_t *pv), but after all the 
2nd argument is only a _pointer_ to an unsigned variable, the size is 
the same (thanks to the C99 explicit types). Qemu will dump this value 
straight into a file, in this case signedness does not matter as long as 
put and get use the same. The more cleaner solution would be to 
introduce an explicitly signed prototype for qemu_put_* and qemu_get_* 
and use this where applicable, if you want I will prepare a patch for this.
If you want some real fix, I can dig deeper and make more changes 
(especially the uint8_t vs. char problem), but I wanted to start slowly 
and don't change much code in the first effort.


What solution do you prefer for the opaque types? I have used the simple:
 -void *args[MAX_ARGS];
 +intptr_t args[MAX_ARGS];
A more portable and clean solution would be this:
-void *args[MAX_ARGS];
+union
+{
+void* ptr;
+int i;
+} args[MAX_ARGS];
If you prefer this, I can change the patch accordingly.

 I will try to do this and apply other parts of your patch
 when I find a bit of time.
Thanks for that, if I can help you with some boring work, tell me ;-)

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

[Qemu-devel] [PATCH 1/5] gcc4 warnings: fix wrong signedness

[Andre Przywara]

This fixes the signedness of some variables to fit the signedness of the
functions called.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
Index: audio/alsaaudio.c
===
RCS file: /sources/qemu/qemu/audio/alsaaudio.c,v
retrieving revision 1.11
diff -p -u -r1.11 alsaaudio.c
--- audio/alsaaudio.c   17 Nov 2007 17:35:54 -  1.11
+++ audio/alsaaudio.c   5 Dec 2007 23:38:57 -
@@ -86,9 +86,9 @@ static struct {
 };
 
 struct alsa_params_req {
-int freq;
+unsigned int freq;
 audfmt_e fmt;
-int nchannels;
+unsigned int nchannels;
 unsigned int buffer_size;
 unsigned int period_size;
 };
@@ -285,7 +285,8 @@ static int alsa_open (int in, struct als
 {
 snd_pcm_t *handle;
 snd_pcm_hw_params_t *hw_params;
-int err, freq, nchannels;
+int err;
+unsigned int freq, nchannels;
 const char *pcm_name = in ? conf.pcm_name_in : conf.pcm_name_out;
 unsigned int period_size, buffer_size;
 snd_pcm_uframes_t obt_buffer_size;
Index: block-vvfat.c
===
RCS file: /sources/qemu/qemu/block-vvfat.c,v
retrieving revision 1.13
diff -p -u -r1.13 block-vvfat.c
--- block-vvfat.c   18 Nov 2007 01:44:35 -  1.13
+++ block-vvfat.c   5 Dec 2007 23:38:57 -
@@ -412,7 +412,7 @@ static void init_mbr(BDRVVVFATState* s)
 /* direntry functions */
 
 /* dest is assumed to hold 258 bytes, and pads with 0x up to next multiple 
of 26 */
-static inline int short2long_name(unsigned char* dest,const char* src)
+static inline int short2long_name(char* dest,const char* src)
 {
 int i;
 for(i=0;i129  src[i];i++) {
Index: gdbstub.c
===
RCS file: /sources/qemu/qemu/gdbstub.c,v
retrieving revision 1.71
diff -p -u -r1.71 gdbstub.c
--- gdbstub.c   17 Nov 2007 17:14:37 -  1.71
+++ gdbstub.c   5 Dec 2007 23:38:57 -
@@ -63,7 +63,7 @@ typedef struct GDBState {
 char line_buf[4096];
 int line_buf_index;
 int line_csum;
-char last_packet[4100];
+uint8_t last_packet[4100];
 int last_packet_len;
 #ifdef CONFIG_USER_ONLY
 int fd;
@@ -188,7 +188,7 @@ static void hextomem(uint8_t *mem, const
 static int put_packet(GDBState *s, char *buf)
 {
 int len, csum, i;
-char *p;
+uint8_t *p;
 
 #ifdef DEBUG_GDB
 printf(reply='%s'\n, buf);
@@ -1179,7 +1179,7 @@ static void gdb_read_byte(GDBState *s, i
 {
 CPUState *env = s-env;
 int i, csum;
-char reply[1];
+uint8_t reply;
 
 #ifndef CONFIG_USER_ONLY
 if (s-last_packet_len) {
@@ -1237,12 +1237,12 @@ static void gdb_read_byte(GDBState *s, i
 csum += s-line_buf[i];
 }
 if (s-line_csum != (csum  0xff)) {
-reply[0] = '-';
-put_buffer(s, reply, 1);
+reply = '-';
+put_buffer(s, reply, 1);
 s-state = RS_IDLE;
 } else {
-reply[0] = '+';
-put_buffer(s, reply, 1);
+reply = '+';
+put_buffer(s, reply, 1);
 s-state = gdb_handle_packet(s, env, s-line_buf);
 }
 break;
Index: hw/fdc.c
===
RCS file: /sources/qemu/qemu/hw/fdc.c,v
retrieving revision 1.33
diff -p -u -r1.33 fdc.c
--- hw/fdc.c17 Nov 2007 17:14:41 -  1.33
+++ hw/fdc.c5 Dec 2007 23:38:57 -
@@ -180,7 +180,7 @@ typedef struct fd_format_t {
 uint8_t last_sect;
 uint8_t max_track;
 uint8_t max_head;
-const unsigned char *str;
+const char *str;
 } fd_format_t;
 
 static const fd_format_t fd_formats[] = {
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -p -u -r1.72 ide.c
--- hw/ide.c18 Nov 2007 01:44:37 -  1.72
+++ hw/ide.c5 Dec 2007 23:38:57 -
@@ -471,12 +470,12 @@ static void ide_identify(IDEState *s)
 put_le16(p + 5, 512); /* XXX: retired, remove ? */
 put_le16(p + 6, s-sectors);
 snprintf(buf, sizeof(buf), QM%05d, s-drive_serial);
-padstr((uint8_t *)(p + 10), buf, 20); /* serial number */
+padstr((char *)(p + 10), buf, 20); /* serial number */
 put_le16(p + 20, 3); /* XXX: retired, remove ? */
 put_le16(p + 21, 512); /* cache size in sectors */
 put_le16(p + 22, 4); /* ecc bytes */
-padstr((uint8_t *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr

[Qemu-devel] [PATCH 2/5] gcc4 warnings: fix char* signedness

[Andre Przywara]

Text strings are char*, buffers are usually uint8_t*, sometimes both are 
mixed, casts are mostly necessary here.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
Index: block-vpc.c
===
RCS file: /sources/qemu/qemu/block-vpc.c,v
retrieving revision 1.7
diff -p -u -r1.7 block-vpc.c
--- block-vpc.c 11 Nov 2007 02:51:16 -  1.7
+++ block-vpc.c 5 Dec 2007 23:38:56 -
@@ -81,7 +81,7 @@ typedef struct BDRVVPCState {
 
 static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
-if (buf_size = 8  !strncmp(buf, conectix, 8))
+if (buf_size = 8  !strncmp((char *)buf, conectix, 8))
return 100;
 return 0;
 }
Index: block-vvfat.c
===
RCS file: /sources/qemu/qemu/block-vvfat.c,v
retrieving revision 1.13
diff -p -u -r1.13 block-vvfat.c
--- block-vvfat.c   18 Nov 2007 01:44:35 -  1.13
+++ block-vvfat.c   5 Dec 2007 23:38:57 -
@@ -565,7 +565,7 @@ static inline uint32_t fat_get(BDRVVVFAT
uint16_t* entry=array_get((s-fat),cluster);
return le16_to_cpu(*entry);
 } else {
-   const uint8_t* x=s-fat.pointer+cluster*3/2;
+   const uint8_t* x=(uint8_t*)(s-fat.pointer)+cluster*3/2;
return ((x[0]|(x[1]8))(cluster1?4:0))0x0fff;
 }
 }
@@ -626,7 +626,7 @@ static inline direntry_t* create_short_a
 
 entry=array_get_next((s-directory));
 memset(entry-name,0x20,11);
-strncpy(entry-name,filename,i);
+strncpy((char*)entry-name,filename,i);
 
 if(j  0)
for (i = 0; i  3  filename[j+1+i]; i++)
@@ -868,7 +868,7 @@ static int init_directories(BDRVVVFATSta
 {
direntry_t* entry=array_get_next((s-directory));
entry-attributes=0x28; /* archive | volume label */
-   snprintf(entry-name,11,QEMU VVFAT);
+   snprintf((char*)entry-name,11,QEMU VVFAT);
 }
 
 /* Now build FAT, and write back information into directory */
@@ -1187,7 +1187,7 @@ static inline int read_cluster(BDRVVVFAT
s-current_mapping = mapping;
 read_cluster_directory:
offset = 
s-cluster_size*(cluster_num-s-current_mapping-begin);
-   s-cluster = s-directory.pointer+offset
+   s-cluster = (unsigned char*)s-directory.pointer+offset
+ 0x20*s-current_mapping-info.dir.first_dir_index;
assert(((s-cluster-(unsigned 
char*)s-directory.pointer)%s-cluster_size)==0);
assert((char*)s-cluster+s-cluster_size = 
s-directory.pointer+s-directory.next*s-directory.item_size);
@@ -1457,7 +1457,7 @@ static int parse_long_name(long_file_nam
 }
 
 if (pointer[0]  0x40)
-   lfn-len = offset + strlen(lfn-name + offset);
+   lfn-len = offset + strlen((char*)lfn-name + offset);
 
 return 0;
 }
@@ -1496,7 +1496,7 @@ static int parse_short_name(BDRVVVFATSta
 } else
lfn-name[i + j + 1] = '\0';
 
-lfn-len = strlen(lfn-name);
+lfn-len = strlen((char*)lfn-name);
 
 return 0;
 }
@@ -1792,8 +1792,8 @@ DLOG(fprintf(stderr, check direntry %d:
fprintf(stderr, Error in short name (%d)\n, subret);
goto fail;
}
-   if (subret  0 || !strcmp(lfn.name, .)
-   || !strcmp(lfn.name, ..))
+   if (subret  0 || !strcmp((char*)lfn.name, .)
+   || !strcmp((char*)lfn.name, ..))
continue;
}
lfn.checksum = 0x100; /* cannot use long name twice */
@@ -1802,7 +1802,7 @@ DLOG(fprintf(stderr, check direntry %d:
fprintf(stderr, Name too long: %s/%s\n, path, lfn.name);
goto fail;
}
-   strcpy(path2 + path_len + 1, lfn.name);
+   strcpy(path2 + path_len + 1, (char*)lfn.name);
 
if (is_directory(direntries + i)) {
if (begin_of_direntry(direntries + i) == 0) {
@@ -2234,7 +2234,7 @@ static int commit_one_file(BDRVVVFATStat
assert(size = 0);
 
ret = vvfat_read(s-bs, cluster2sector(s, c),
-   cluster, (rest_size + 0x1ff) / 0x200);
+   (uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
 
if (ret  0)
return ret;
Index: gdbstub.c
===
RCS file: /sources/qemu/qemu/gdbstub.c,v
retrieving revision 1.71
diff -p -u -r1.71 gdbstub.c
--- gdbstub.c   17 Nov 2007 17:14:37 -  1.71
+++ gdbstub.c   5 Dec 2007 23:38:57 -
@@ -209,7 +209,7 @@ static int put_packet(GDBState

[Qemu-devel] [PATCH 0/5] fix various gcc4 compile warnings

[Andre Przywara]

Hi,

after my first try last week now a reworked version of the patches to
fix (gcc4) compiler warnings. These warnings occur when compiling for
instance the embedded QEmu in Xen.
I splitted the big patch to address several issues separately:
1/5: fix wrong types (wrong signedness)
2/5: fix char* signedness
3/5: qemu_put signedness fixes
4/5: fix bdrv_get_geometry to return uint64_t
5/5: miscellaneous minor things

Compared to my last work post I tried to avoid casting as much as
possible, only patch 2/5 still uses them.
Comments welcome.

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x84917

[Qemu-devel] [PATCH 5/5] gcc4 warnings: miscellaneous minor things

[Andre Przywara]

some things found on the way:
- missing include in hw/adlib.c
- avoid double definition of NDEBUG in aes.h
- fix some strange pointer acrobatics in hw/ide.c#padstr
- fix getsockopt socklen_t warning
- fix uninitialized variable warning in monitor.c
- fix wrong signedness in le16_to_cpus calls in hw/pcnet.c

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
Index: aes.c
===
RCS file: /sources/qemu/qemu/aes.c,v
retrieving revision 1.4
diff -p -u -r1.4 aes.c
--- aes.c   11 Nov 2007 02:51:15 -  1.4
+++ aes.c   5 Dec 2007 23:38:56 -
@@ -30,7 +30,10 @@
 #include qemu-common.h
 #include aes.h
 
+#ifndef NDEBUG
 #define NDEBUG
+#endif
+
 #include assert.h
 
 typedef uint32_t u32;
Index: hw/adlib.c
===
RCS file: /sources/qemu/qemu/hw/adlib.c,v
retrieving revision 1.10
diff -p -u -r1.10 adlib.c
--- hw/adlib.c  2 Dec 2007 17:47:33 -   1.10
+++ hw/adlib.c  5 Dec 2007 23:38:57 -
@@ -26,6 +26,7 @@
 #include hw.h
 #include audiodev.h
 #include audio/audio.h
+#include isa.h
 
 //#define DEBUG
 
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -p -u -r1.72 ide.c
--- hw/ide.c18 Nov 2007 01:44:37 -  1.72
+++ hw/ide.c5 Dec 2007 23:38:57 -
@@ -430,8 +430,7 @@ static void padstr(char *str, const char
 v = *src++;
 else
 v = ' ';
-*(char *)((long)str ^ 1) = v;
-str++;
+str[i^1] = v;
 }
 }
 
Index: hw/pcnet.c
===
RCS file: /sources/qemu/qemu/hw/pcnet.c,v
retrieving revision 1.20
diff -p -u -r1.20 pcnet.c
--- hw/pcnet.c  17 Nov 2007 17:14:45 -  1.20
+++ hw/pcnet.c  5 Dec 2007 23:38:58 -
@@ -350,8 +350,8 @@ static inline void pcnet_tmd_load(PCNetS
 } else {
 s-phys_mem_read(s-dma_opaque, addr, (void *)tmd, sizeof(*tmd), 0);
 le32_to_cpus(tmd-tbadr);
-le16_to_cpus(tmd-length);
-le16_to_cpus(tmd-status);
+le16_to_cpus((uint16_t *)tmd-length);
+le16_to_cpus((uint16_t *)tmd-status);
 le32_to_cpus(tmd-misc);
 le32_to_cpus(tmd-res);
 if (BCR_SWSTYLE(s) == 3) {
@@ -416,8 +416,8 @@ static inline void pcnet_rmd_load(PCNetS
 } else {
 s-phys_mem_read(s-dma_opaque, addr, (void *)rmd, sizeof(*rmd), 0);
 le32_to_cpus(rmd-rbadr);
-le16_to_cpus(rmd-buf_length);
-le16_to_cpus(rmd-status);
+le16_to_cpus((uint16_t *)rmd-buf_length);
+le16_to_cpus((uint16_t *)rmd-status);
 le32_to_cpus(rmd-msg_length);
 le32_to_cpus(rmd-res);
 if (BCR_SWSTYLE(s) == 3) {
Index: monitor.c
===
RCS file: /sources/qemu/qemu/monitor.c,v
retrieving revision 1.91
diff -p -u -r1.91 monitor.c
--- monitor.c   3 Dec 2007 17:05:38 -   1.91
+++ monitor.c   5 Dec 2007 23:38:57 -
@@ -1824,7 +1824,7 @@ static int64_t expr_unary(void)
 case '$':
 {
 char buf[128], *q;
-target_long reg;
+target_long reg=0;
 
 pch++;
 q = buf;
Index: vl.c
===
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.376
diff -p -u -r1.376 vl.c
--- vl.c4 Dec 2007 00:10:34 -   1.376
+++ vl.c5 Dec 2007 23:38:57 -
@@ -4397,7 +4398,8 @@ static NetSocketState *net_socket_fd_ini
 {
 int so_type=-1, optlen=sizeof(so_type);
 
-if(getsockopt(fd, SOL_SOCKET, SO_TYPE, (char *)so_type, optlen) 0) {
+if(getsockopt(fd, SOL_SOCKET, SO_TYPE, (char *)so_type,
+(socklen_t *)optlen) 0) {
fprintf(stderr, qemu: error: getsockopt(SO_TYPE) for fd=%d failed\n, 
fd);
return NULL;
 }

[Qemu-devel] [PATCH 4/5] gcc4 warnings: fix bdrv_get_geometry to return uint64_t

[Andre Przywara]

bdrv_get_geometry never returns a negative number, so I changed the 
return type to unsigned, changes quite a lot of declarations.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
Index: block.c
===
RCS file: /sources/qemu/qemu/block.c,v
retrieving revision 1.50
diff -p -u -r1.50 block.c
--- block.c 2 Dec 2007 05:18:18 -   1.50
+++ block.c 5 Dec 2007 23:38:57 -
@@ -717,7 +717,7 @@ int64_t bdrv_getlength(BlockDriverState 
 }
 
 /* return 0 as number of sectors if no device present or error */
-void bdrv_get_geometry(BlockDriverState *bs, int64_t *nb_sectors_ptr)
+void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
 {
 int64_t length;
 length = bdrv_getlength(bs);
Index: block.h
===
RCS file: /sources/qemu/qemu/block.h,v
retrieving revision 1.3
diff -p -u -r1.3 block.h
--- block.h 2 Dec 2007 05:18:18 -   1.3
+++ block.h 5 Dec 2007 23:38:57 -
@@ -72,7 +72,7 @@ int bdrv_pwrite(BlockDriverState *bs, in
 const void *buf, int count);
 int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
-void bdrv_get_geometry(BlockDriverState *bs, int64_t *nb_sectors_ptr);
+void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 int bdrv_commit(BlockDriverState *bs);
 void bdrv_set_boot_sector(BlockDriverState *bs, const uint8_t *data, int size);
 /* async block I/O */
Index: hw/fdc.c
===
RCS file: /sources/qemu/qemu/hw/fdc.c,v
retrieving revision 1.33
diff -p -u -r1.33 fdc.c
--- hw/fdc.c17 Nov 2007 17:14:41 -  1.33
+++ hw/fdc.c5 Dec 2007 23:38:57 -
@@ -234,7 +234,7 @@ static const fd_format_t fd_formats[] = 
 static void fd_revalidate (fdrive_t *drv)
 {
 const fd_format_t *parse;
-int64_t nb_sectors, size;
+uint64_t nb_sectors, size;
 int i, first_match, match;
 int nb_heads, max_track, last_sect, ro;
 
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -p -u -r1.72 ide.c
--- hw/ide.c18 Nov 2007 01:44:37 -  1.72
+++ hw/ide.c5 Dec 2007 23:38:57 -
@@ -1465,12 +1464,12 @@ static void ide_atapi_cmd(IDEState *s)
 break;
 case GPCMD_SEEK:
 {
-int lba;
-int64_t total_sectors;
+unsigned int lba;
+uint64_t total_sectors;
 
 bdrv_get_geometry(s-bs, total_sectors);
 total_sectors = 2;
-if (total_sectors = 0) {
+if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY,
 ASC_MEDIUM_NOT_PRESENT);
 break;
@@ -1516,11 +1515,11 @@ static void ide_atapi_cmd(IDEState *s)
 case GPCMD_READ_TOC_PMA_ATIP:
 {
 int format, msf, start_track, len;
-int64_t total_sectors;
+uint64_t total_sectors;
 
 bdrv_get_geometry(s-bs, total_sectors);
 total_sectors = 2;
-if (total_sectors = 0) {
+if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY,
 ASC_MEDIUM_NOT_PRESENT);
 break;
@@ -1560,11 +1559,11 @@ static void ide_atapi_cmd(IDEState *s)
 break;
 case GPCMD_READ_CDVD_CAPACITY:
 {
-int64_t total_sectors;
+uint64_t total_sectors;
 
 bdrv_get_geometry(s-bs, total_sectors);
 total_sectors = 2;
-if (total_sectors = 0) {
+if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY,
 ASC_MEDIUM_NOT_PRESENT);
 break;
@@ -1580,7 +1579,7 @@ static void ide_atapi_cmd(IDEState *s)
 int media = packet[1];
 int layer = packet[6];
 int format = packet[2];
-int64_t total_sectors;
+uint64_t total_sectors;
 
 if (media != 0 || layer != 0)
 {
@@ -1592,6 +1591,11 @@ static void ide_atapi_cmd(IDEState *s)
 case 0:
 bdrv_get_geometry(s-bs, total_sectors);
 total_sectors = 2;
+if (total_sectors == 0) {
+ide_atapi_cmd_error(s, SENSE_NOT_READY

[Qemu-devel] [PATCH 3/5] gcc4 warnings: qemu_put signedness fixes

[Andre Przywara]

qemu_{put|get}_{16,32,64}s takes an unsigned pointer, signed variables 
produce a warning. Since most of the variables are actually ints, I 
reverted to use qemu_{put|get}_{16,32,64} (without the 's' and using the 
value instead of the pointer). This fixes those warnings. I couldn't get 
the point why only the pointer version was used throughout all files, so 
I expect some resistance here...


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
Index: hw/acpi.c
===
RCS file: /sources/qemu/qemu/hw/acpi.c,v
retrieving revision 1.17
diff -p -u -r1.17 acpi.c
--- hw/acpi.c   17 Nov 2007 17:14:39 -  1.17
+++ hw/acpi.c   5 Dec 2007 23:38:57 -
@@ -439,7 +439,7 @@ static void pm_save(QEMUFile* f,void *op
 qemu_put_8s(f, s-apmc);
 qemu_put_8s(f, s-apms);
 qemu_put_timer(f, s-tmr_timer);
-qemu_put_be64s(f, s-tmr_overflow_time);
+qemu_put_be64(f, s-tmr_overflow_time);
 }
 
 static int pm_load(QEMUFile* f,void* opaque,int version_id)
@@ -460,7 +460,7 @@ static int pm_load(QEMUFile* f,void* opa
 qemu_get_8s(f, s-apmc);
 qemu_get_8s(f, s-apms);
 qemu_get_timer(f, s-tmr_timer);
-qemu_get_be64s(f, s-tmr_overflow_time);
+s-tmr_overflow_time=qemu_get_be64(f);
 
 pm_io_space_update(s);
 
Index: hw/apic.c
===
RCS file: /sources/qemu/qemu/hw/apic.c,v
retrieving revision 1.18
diff -p -u -r1.18 apic.c
--- hw/apic.c   17 Nov 2007 17:14:40 -  1.18
+++ hw/apic.c   5 Dec 2007 23:38:57 -
@@ -761,10 +761,10 @@ static void apic_save(QEMUFile *f, void 
 qemu_put_be32s(f, s-icr[0]);
 qemu_put_be32s(f, s-icr[1]);
 qemu_put_be32s(f, s-divide_conf);
-qemu_put_be32s(f, s-count_shift);
+qemu_put_be32(f, s-count_shift);
 qemu_put_be32s(f, s-initial_count);
-qemu_put_be64s(f, s-initial_count_load_time);
-qemu_put_be64s(f, s-next_time);
+qemu_put_be64(f, s-initial_count_load_time);
+qemu_put_be64(f, s-next_time);
 
 qemu_put_timer(f, s-timer);
 }
@@ -797,10 +797,10 @@ static int apic_load(QEMUFile *f, void *
 qemu_get_be32s(f, s-icr[0]);
 qemu_get_be32s(f, s-icr[1]);
 qemu_get_be32s(f, s-divide_conf);
-qemu_get_be32s(f, s-count_shift);
+s-count_shift=qemu_get_be32(f);
 qemu_get_be32s(f, s-initial_count);
-qemu_get_be64s(f, s-initial_count_load_time);
-qemu_get_be64s(f, s-next_time);
+s-initial_count_load_time=qemu_get_be64(f);
+s-next_time=qemu_get_be64(f);
 
 if (version_id = 2)
 qemu_get_timer(f, s-timer);
Index: hw/cirrus_vga.c
===
RCS file: /sources/qemu/qemu/hw/cirrus_vga.c,v
retrieving revision 1.29
diff -p -u -r1.29 cirrus_vga.c
--- hw/cirrus_vga.c 17 Nov 2007 17:14:40 -  1.29
+++ hw/cirrus_vga.c 5 Dec 2007 23:38:57 -
@@ -2985,7 +2985,7 @@ static void cirrus_vga_save(QEMUFile *f,
 qemu_put_buffer(f, s-gr + 2, 254);
 qemu_put_8s(f, s-ar_index);
 qemu_put_buffer(f, s-ar, 21);
-qemu_put_be32s(f, s-ar_flip_flop);
+qemu_put_be32(f, s-ar_flip_flop);
 qemu_put_8s(f, s-cr_index);
 qemu_put_buffer(f, s-cr, 256);
 qemu_put_8s(f, s-msr);
@@ -3000,7 +3000,7 @@ static void cirrus_vga_save(QEMUFile *f,
 qemu_put_buffer(f, s-dac_cache, 3);
 qemu_put_buffer(f, s-palette, 768);
 
-qemu_put_be32s(f, s-bank_offset);
+qemu_put_be32(f, s-bank_offset);
 
 qemu_put_8s(f, s-cirrus_hidden_dac_lockindex);
 qemu_put_8s(f, s-cirrus_hidden_dac_data);
@@ -3036,7 +3036,7 @@ static int cirrus_vga_load(QEMUFile *f, 
 qemu_get_buffer(f, s-gr + 2, 254);
 qemu_get_8s(f, s-ar_index);
 qemu_get_buffer(f, s-ar, 21);
-qemu_get_be32s(f, s-ar_flip_flop);
+s-ar_flip_flop=qemu_get_be32(f);
 qemu_get_8s(f, s-cr_index);
 qemu_get_buffer(f, s-cr, 256);
 qemu_get_8s(f, s-msr);
@@ -3051,7 +3051,7 @@ static int cirrus_vga_load(QEMUFile *f, 
 qemu_get_buffer(f, s-dac_cache, 3);
 qemu_get_buffer(f, s-palette, 768);
 
-qemu_get_be32s(f, s-bank_offset);
+s-bank_offset=qemu_get_be32(f);
 
 qemu_get_8s(f, s-cirrus_hidden_dac_lockindex);
 qemu_get_8s(f, s-cirrus_hidden_dac_data);
Index: hw/dma.c
===
RCS file: /sources/qemu/qemu/hw/dma.c,v
retrieving revision 1.16
diff -p -u -r1.16 dma.c
--- hw/dma.c17 Nov 2007 17:14:41 -  1.16
+++ hw/dma.c5 Dec 2007 23:38:57 -
@@ -482,12 +482,12 @@ static void dma_save (QEMUFile *f, void 
 qemu_put_8s (f, d-command

Re: [Qemu-devel] Re: [kvm-ppc-devel] The default for char Literals differ in signedness between platforms causing us a lot of warnings

[Andre Przywara]

Christian,

 Newer gcc versions generate warnings about implicit casts between
 different signed pointers.
 That hits a lot of qemu code at least what I saw it compiling for ppc
 or x86.
 So my question is, is there already a preferred qemu approach to get
 rid of these warnings
 either the -Wno-pointer-sign solution like in the kernel or something
 else (I did not find
 anything like that in the latest cvs snapshot or on this list)?

I fixed some of these issues in qemu at least for x86[1][2], those were 
commited mid of December[3] . I ported these over to Xen and posted it 
there[4]. I don't know what's the ultimate policy regarding these 
problems, my solution is:

1. Change the type whenever possible.
2. Introduce casts if 1. is not applicable or would introduce more 
back-castings.


Regards,
Andre.

[1] QEMU first proposal (all in one file) with comments:
http://lists.gnu.org/archive/html/qemu-devel/2007-11/msg00827.html
[2] QEMU reworked version (splitted up to address multiple issues)
http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00146.html
[3] QEMU commits:
http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00417.html
http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00420.html
http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00448.html
http://lists.gnu.org/archive/html/qemu-devel/2007-12/msg00452.html
[4] Xen devel post 
http://lists.xensource.com/archives/html/xen-devel/2008-01/msg00185.html


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [Qemu-devel] [PATCH][RFC] To mount qemu disk image on the host

[Andre Przywara]

Laurent Vivier wrote:

Le vendredi 25 janvier 2008 à 09:18 -0600, Anthony Liguori a écrit :

Laurent Vivier wrote:

Hi,

this patch allows to mount qemu disk images on the host.
  


Sorry, I didn't see you did a similar work 19 months ago.
Note, the general problem with this approach is that mounting a NBD 
device locally with write access can lead to dead locks.  If you look 
through the mailing list archives, you'll find a number of conversations 
on the topic.
I sometimes ago was also working on a nbd implementation for 
qcow-images, but I came to the same deadlock conclusion. (At least 
theoretically, I didn't finish this as I ran first into debugging 
problems and secondly out of time). But IMHO this only applies to 
localhost mounts, real network mounting should work (this is actually 
not different from native nbd). Perhaps one could use a qemu instance 
for the server part ;-)
BTW: nbd-server should be quite portable, I once had it run on an 
ancient PA-RISC machine under HP-UX 10.20.



What I'm wondering is how loop and device mapper can work ?
I shortly evaluated the loop device idea, but came to the conclusion 
that this not so easy to implement (and would require qcow code in the 
kernel). I see only little chance for this go upstream in Linux and 
maintaining this out-of-tree is actually a bad idea.
If you think about deferring the qcow code into userland, you will 
sooner or later run into the same deadlock problems as the current 
solution (after all this is what nbd does...)


I have implemented a clean device-mapper solution, the big drawback is 
that it is read-only. It's a simple tool which converts the qcow map 
into a format suitable for dm-setup, to which the output can be directly 
piped to. I will clean up the code and send it to the list ASAP.
Read/write support is not that easy, but maybe someone can comment on 
this idea:
Create a sparse file on the host which is as large as the number of all 
still unallocated blocks. Assign these blocks via device mapper in 
addition to the already allocated ones. When unmounting the dm device, 
look for blocks which have been changed and allocate and write them into 
the qcow file. One could also use the bmap-ioctl to scan for non-sparse 
blocks.
This is a bit complicated, but should work cleanly (especially for the 
quick fsck or file editing case). If you find it worth, I could try to 
implement it.


Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
to satisfy European Law for business letters:
AMD Saxony Limited Liability Company  Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

[Qemu-devel] Re: CPU type qemu64 breaks guest code

[Andre Przywara]

On 03/14/2011 03:13 PM, Alexander Graf wrote:

Hi guys,

While I was off on vacation a pretty nasty bug emerged. It's our old
friend the non-existent -cpu qemu64 CPU type. To refresh your memories,
this is the definition of the default 64-bit CPU type in Qemu:

 {
 .name = qemu64,
 .level = 4,
 .vendor1 = CPUID_VENDOR_AMD_1,
 .vendor2 = CPUID_VENDOR_AMD_2,
 .vendor3 = CPUID_VENDOR_AMD_3,
 .family = 6,
 .model = 2,
 .stepping = 3,

 ...

Well, yes, this is strange, and a different CPU model mimicking some 
really existing CPU would be better. But in my experience this opens up 
a can of worms, since a different family will trigger a lot of other 
nasty code that was silent before. Although I think that eventually we 
will not get around it doing so, but we should use a lot of testing 
before triggering tons of regressions.
What about the kvm64 CPU model? Back then I used quite some testing to 
find a model which runs pretty well, so I came up with 15/6/1, which 
seemed to be relatively sane. We could try copying this F/M/S over to 
qemu64, I suppose with emulation the issues are easier to fix than in KVM.


Another idea I think I already posted some time ago was to make kvm64 
the new default if KVM is enabled. This wouldn't solve the above issue 
for TCG of course, but would make further development easier, since we 
would have a better separation between KVM and TCG and could tweak each 
independently.



Before I left, we already had weird build breakages where gcc simply

 abort()ed when running inside of KVM. This breakage has been tracked

down to libgmp, which has this code
(http://gmplib.org:8000/gmp-5.0/file/1ebe39104437/mpn/x86_64/fat/fat.c):

 



   if (strcmp (vendor_string, GenuineIntel) == 0)
 {
   
 }
   else if (strcmp (vendor_string, AuthenticAMD) == 0)
 {
   switch (family)
 {
case 5:
case 6:
abort ();
break;
case 15:
   /* CPUVEC_SETUP_athlon */
   break;
 }

The reasoning behind it is that for AMD, all 64-bit CPUs were family
15.


I guess that should be fixed there, as it is obviously nonsense. I 
haven't check what they actually need that family 6 does not provide, if 
it is long mode, then this bit should be checked.



This breakage is obviously pretty bad for guests running qemu and
shows once again that deriving from real hardware is a bad idea. I guess
the best way to move forward is to change the CPU type to something that
actually exists in the real world - even if we have to strip off some
features.


I found that there is no valid combination for both Intel and AMD. We 
had to go to family 15, and it seems that there is no F/M/S combination 
that were both a valid K8 and a Pentium4 processor. The 15/6/1 mentioned 
before was the closest match I could find.


Summarizing I would suggest:
1) Make kvm64 the new default model if KVM is used. Maybe we could tie
   this to the qemu-0.15 machine type.
2) Test as many OSes as possible whether they show strange behavior.
   In my experience we could catch most of the stuff with just booting
   the system, with Linux -kernel vmlinuz suffices (without a rootfs).
3) If we are happy with that, tweak the qemu64 model to those values,
   too.
4) Write some note somewhere to let users know what they could do to
   fix problems that rise due to the new model.

I can easily send patches and will try to contribute to testing, if that 
plan sounds OK.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [Qemu-devel] [PATCH resend] vl.c: Don't limit node count by smp count

[Andre Przywara]

On 06/30/2011 05:29 AM, Sasha Levin wrote:

[I've sent this patch couple of months ago and noticed it
  didn't make it's way in - so I'm sending it again]

It is possible to create CPU-less NUMA nodes, node amount shouldn't be
limited by amount of CPUs.

Tested-by: Michael Rothmdr...@linux.vnet.ibm.com
Signed-off-by: Sasha Levinlevinsasha...@gmail.com
---
  vl.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index e0191e1..b95ae8d 100644
--- a/vl.c
+++ b/vl.c
@@ -3147,8 +3147,8 @@ int main(int argc, char **argv, char **envp)
  if (nb_numa_nodes  0) {
  int i;

-if (nb_numa_nodes  smp_cpus) {
-nb_numa_nodes = smp_cpus;
+if (nb_numa_nodes  MAX_NODES) {
+nb_numa_nodes = MAX_NODES;
  }

  /* If no memory size if given for any node, assume the default case


Acked-by: Andre Przywara andre.przyw...@amd.com

Regards,
Andre.


--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [Qemu-devel] qemu -numa option and non-contiguous CPU ranges

[Andre Przywara]

On 06/21/2012 07:51 PM, Eduardo Habkost wrote:

Hi,

I just noticed libvirt tries to use the -numa option in a way that qemu
never understood: if a node is configured to have a non-contiguous set
of CPUs, it tries to generate a command-line option that looks like:

-numa node,nodeid=...,cpus=0,2,4,mem=...
 ^

But this format was never supported by qemu. This format is even a bit
weird, as , is an option separator, and it is being used as a
separator _inside_ an option.


Exactly this was the reason back then to not support non-contiguous set 
of CPUs. Inside qemu there is no reason why this shouldn't work, it was 
just hard to write on the command line. So after a short discussion we 
decided to drop this for the time being. If you have a great idea how to 
specify this (I think a comma will not work, because it will be catched 
earlier), I am all ears.


Regards,
Andre.


My question is: should we support this option format in qemu, or should
we change libvirt to use another format (that has yet to be implemented,
because currently there's no way to specify a non-contiguous set of CPUs
for a NUMA node).

Any suggestions?




--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [Qemu-devel] Semantics of -cpu host (was Re: [PATCH 2/2] Expose tsc deadline timer cpuid to guest)

[Andre Przywara]

On 05/07/2012 08:21 PM, Eduardo Habkost wrote:


Andre? Are you able to help to answer the question below?


Sorry for the delay, the easy answers first:


I would like to clarify what's the expected behavior of -cpu host to
be able to continue working on it.


The purpose of -cpu host is to let guests use ISA features that the host 
CPU provides. Those would not need any KVM/QEMU intervention, because 
they work out of the box. Things like AES or SSE4.2, which are used by 
Linux and glibc, for instance. Users would expect those to be usable, 
and actually we only need to set the bits and are done.


A second goal is to get rid of the awkward and artificial 
family/model/stepping settings and just let the guest see the actual CPU 
model. This has some strings attached, though, but other virtualization 
solution do it the same way and they can cope with it.


A third thing currently not really addressed are the more informational 
bits like cache size and organization and TLB layout. Those are 
properties of the (core) CPU (except shared L3, cache maybe) and apply 
to guests as well. I don't see any reason why this should not be exposed 
to the guest. From the top of my head I don't know any prominent user 
(just glibc reading the cache size, but not really using it), but I 
guess there are applications which benefit from it.


What clearly is not the intention of -cpu host is to provide access to 
every feature a certain CPU model provides. So things which require 
emulation or hypervisor intervention are not in the primary use case. 
That does not mean that they don't need to implemented, but that was not 
the purpose of -cpu host and they should be handled independently.


Regards,
Andre.

I believe the code will need to be

fixed on either case, but first we need to figure out what are the
expectations/requirements, to know _which_ changes will be needed.


On Tue, Apr 24, 2012 at 02:19:25PM -0300, Eduardo Habkost wrote:

(CCing Andre Przywara, in case he can help to clarify what's the
expected meaning of -cpu host)


[...]

I am not sure I understand what you are proposing. Let me explain the
use case I am thinking about:

- Feature FOO is of type (A) (e.g. just a new instruction set that
   doesn't require additional userspace support)
- User has a Qemu vesion that doesn't know anything about feature FOO
- User gets a new CPU that supports feature FOO
- User gets a new kernel that supports feature FOO (i.e. has FOO in
   GET_SUPPORTED_CPUID)
- User does _not_ upgrade Qemu.
- User expects to get feature FOO enabled if using -cpu host, without
   upgrading Qemu.

The problem here is: to support the above use-case, userspace need a
probing mechanism that can differentiate _new_ (previously unknown)
features that are in group (A) (safe to blindly enable) from features
that are in group (B) (that can't be enabled without an userspace
upgrade).

In short, it becomes a problem if we consider the following case:

- Feature BAR is of type (B) (it can't be enabled without extra
   userspace support)
- User has a Qemu version that doesn't know anything about feature BAR
- User gets a new CPU that supports feature BAR
- User gets a new kernel that supports feature BAR (i.e. has BAR in
   GET_SUPPORTED_CPUID)
- User does _not_ upgrade Qemu.
- User simply shouldn't get feature BAR enabled, even if using -cpu
   host, otherwise Qemu would break.

If userspace always limited itself to features it knows about, it would
be really easy to implement the feature without any new probing
mechanism from the kernel. But that's not how I think users expect -cpu
host to work. Maybe I am wrong, I don't know. I am CCing Andre, who
introduced the -cpu host feature, in case he can explain what's the
expected semantics on the cases above.






--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

[Qemu-devel] [PATCH] i386/cpu: name new CPUID bits

[Andre Przywara]

Update QEMU's knowledge of CPUID bit names. This allows to
enable/disable those new features on QEMU's command line when
using KVM and prepares future feature enablement in QEMU.

This adds F16C, RDRAND, LWP, TBM, TopoExt, PerfCtr_Core, PerfCtr_NB,
FSGSBASE, BMI1, AVX2, BMI2, ERMS, InvPCID, RTM, RDSeed and ADX.

Sources where the AMD BKDG for Family 15h/Model 10h and the Linux kernel
for the leaf 7 bits.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 target-i386/cpu.c | 16 
 target-i386/cpu.h | 21 +
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f3708e6..49f9561 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -59,7 +59,7 @@ static const char *ext_feature_name[] = {
 NULL, pcid, dca, sse4.1|sse4_1,
 sse4.2|sse4_2, x2apic, movbe, popcnt,
 tsc-deadline, aes, xsave, osxsave,
-avx, NULL, NULL, hypervisor,
+avx, f16c, rdrand, hypervisor,
 };
 /* Feature names that are already defined on feature_name[] but are set on
  * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
@@ -79,10 +79,10 @@ static const char *ext3_feature_name[] = {
 lahf_lm /* AMD LahfSahf */, cmp_legacy, svm, extapic /* AMD 
ExtApicSpace */,
 cr8legacy /* AMD AltMovCr8 */, abm, sse4a, misalignsse,
 3dnowprefetch, osvw, ibs, xop,
-skinit, wdt, NULL, NULL,
-fma4, NULL, cvt16, nodeid_msr,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
+skinit, wdt, NULL, lwp,
+fma4, tce, NULL, nodeid_msr,
+NULL, tbm, topoext, perfctr_core,
+perfctr_nb, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 };
 
@@ -105,9 +105,9 @@ static const char *svm_feature_name[] = {
 };
 
 static const char *cpuid_7_0_ebx_feature_name[] = {
-NULL, NULL, NULL, NULL, NULL, NULL, NULL, smep,
-NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL, smap, NULL, NULL, NULL,
+fsgsbase, NULL, NULL, bmi1, hle, avx2, NULL, smep,
+bmi2, erms, invpcid, rtm, NULL, NULL, NULL, NULL,
+NULL, NULL, rdseed, adx, smap, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 871c270..e496b5b 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -403,6 +403,7 @@
 #define CPUID_EXT_TM2  (1  8)
 #define CPUID_EXT_SSSE3(1  9)
 #define CPUID_EXT_CID  (1  10)
+#define CPUID_EXT_FMA  (1  12)
 #define CPUID_EXT_CX16 (1  13)
 #define CPUID_EXT_XTPR (1  14)
 #define CPUID_EXT_PDCM (1  15)
@@ -417,6 +418,8 @@
 #define CPUID_EXT_XSAVE(1  26)
 #define CPUID_EXT_OSXSAVE  (1  27)
 #define CPUID_EXT_AVX  (1  28)
+#define CPUID_EXT_F16C (1  29)
+#define CPUID_EXT_RDRAND   (1  30)
 #define CPUID_EXT_HYPERVISOR  (1  31)
 
 #define CPUID_EXT2_FPU (1  0)
@@ -472,7 +475,15 @@
 #define CPUID_EXT3_IBS (1  10)
 #define CPUID_EXT3_XOP (1  11)
 #define CPUID_EXT3_SKINIT  (1  12)
+#define CPUID_EXT3_WDT (1  13)
+#define CPUID_EXT3_LWP (1  15)
 #define CPUID_EXT3_FMA4(1  16)
+#define CPUID_EXT3_TCE (1  17)
+#define CPUID_EXT3_NODEID  (1  19)
+#define CPUID_EXT3_TBM (1  21)
+#define CPUID_EXT3_TOPOEXT (1  22)
+#define CPUID_EXT3_PERFCORE (1  23)
+#define CPUID_EXT3_PERFNB  (1  24)
 
 #define CPUID_SVM_NPT  (1  0)
 #define CPUID_SVM_LBRV (1  1)
@@ -485,7 +496,17 @@
 #define CPUID_SVM_PAUSEFILTER  (1  10)
 #define CPUID_SVM_PFTHRESHOLD  (1  12)
 
+#define CPUID_7_0_EBX_FSGSBASE (1  0)
+#define CPUID_7_0_EBX_BMI1 (1  3)
+#define CPUID_7_0_EBX_HLE  (1  4)
+#define CPUID_7_0_EBX_AVX2 (1  5)
 #define CPUID_7_0_EBX_SMEP (1  7)
+#define CPUID_7_0_EBX_BMI2 (1  8)
+#define CPUID_7_0_EBX_ERMS (1  9)
+#define CPUID_7_0_EBX_INVPCID  (1  10)
+#define CPUID_7_0_EBX_RTM  (1  11)
+#define CPUID_7_0_EBX_RDSEED   (1  18)
+#define CPUID_7_0_EBX_ADX  (1  19)
 #define CPUID_7_0_EBX_SMAP (1  20)
 
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* Genu */
-- 
1.7.12.1

[Qemu-devel] [PATCH] vnc-tls: Fix compilation with newer versions of GNU-TLS

[Andre Przywara]

In my installation of GNU-TLS (v3.0.23) the type
gnutls_anon_server_credentials is marked deprecated, so -Werror
breaks compilation.
Simply replacing it with the newer ..._t version fixed the compilation
on my machine (Slackware 14.0). I cannot tell how far back this new
type goes, at least the header file in RHEL 5.0 (v1.4.1) seems to have
it already. If someone finds a broken distribution, tell me and I
insert some compat code.

Signed-off-by: Andre Przywara andre.przyw...@amd.com
---
 ui/vnc-tls.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/vnc-tls.c b/ui/vnc-tls.c
index a7f7d07..ba3827b 100644
--- a/ui/vnc-tls.c
+++ b/ui/vnc-tls.c
@@ -99,9 +99,9 @@ static ssize_t vnc_tls_pull(gnutls_transport_ptr_t transport,
 }
 
 
-static gnutls_anon_server_credentials vnc_tls_initialize_anon_cred(void)
+static gnutls_anon_server_credentials_t vnc_tls_initialize_anon_cred(void)
 {
-gnutls_anon_server_credentials anon_cred;
+gnutls_anon_server_credentials_t anon_cred;
 int ret;
 
 if ((ret = gnutls_anon_allocate_server_credentials(anon_cred))  0) {
@@ -382,7 +382,7 @@ int vnc_tls_client_setup(struct VncState *vs,
 }
 
 } else {
-gnutls_anon_server_credentials anon_cred = 
vnc_tls_initialize_anon_cred();
+gnutls_anon_server_credentials_t anon_cred = 
vnc_tls_initialize_anon_cred();
 if (!anon_cred) {
 gnutls_deinit(vs-tls.session);
 vs-tls.session = NULL;
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH] i386/cpu: name new CPUID bits

[Andre Przywara]

On 10/18/12 18:33, Eduardo Habkost wrote:

On Wed, Oct 17, 2012 at 11:17:26PM +0200, Andre Przywara wrote:

Update QEMU's knowledge of CPUID bit names. This allows to
enable/disable those new features on QEMU's command line when
using KVM and prepares future feature enablement in QEMU.

...

@@ -79,10 +79,10 @@ static const char *ext3_feature_name[] = {
  lahf_lm /* AMD LahfSahf */, cmp_legacy, svm, extapic /* AMD 
ExtApicSpace */,
  cr8legacy /* AMD AltMovCr8 */, abm, sse4a, misalignsse,
  3dnowprefetch, osvw, ibs, xop,
-skinit, wdt, NULL, NULL,
-fma4, NULL, cvt16, nodeid_msr,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
+skinit, wdt, NULL, lwp,
+fma4, tce, NULL, nodeid_msr,


You removed cvt16, here.


On purpose, cvt16 is dead. It seems to have been advertised in the 
documentation for some time, but later disappeared. The respective 
instructions are now reported via the F16C flag in the Intel leaf.


If you want to know more, I can dig deeper and ask some people. But the 
bit is now reserved and was never '1' in any silicon. (And was 
introduced by me into QEMU :-(




All the rest of the flags look OK to me.


Thanks.

Regards,
Andre.

[Qemu-devel] [PATCH] fdt: update embedded header file from upstream to fix compilation

[Andre Przywara]

Upstream dtc.git introduced a change in libfdt_env.h, which breaks
compilation with QEMU's version of it:

  CC arm-softmmu/device_tree.o
In file included from /usr/include/libfdt.h:55:0,
 from /src/qemu.git/device_tree.c:28:
/usr/include/fdt.h:7:2: error: unknown type name 'fdt32_t'
   ...

The culprit is:
commit feafcd972cb744750a65728440c99526e6199a6d
Author: Kim Phillips kim.phill...@freescale.com
Date:   Wed Nov 28 17:33:01 2012 -0600

dtc/libfdt: introduce fdt types for annotation by endian checkers
...

Pull the new definitions into QEMU's version of the file. This change
also works with older installed versions of dtc.
The upstream version got a GPL or BSD dual license header meanwhile.
I retained the original GPL license header from QEMU, only added
the original copyrights.

Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
 include/libfdt_env.h | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/include/libfdt_env.h b/include/libfdt_env.h
index 3667d4c..aad54bb 100644
--- a/include/libfdt_env.h
+++ b/include/libfdt_env.h
@@ -1,4 +1,12 @@
+#ifndef _LIBFDT_ENV_H
+#define _LIBFDT_ENV_H
 /*
+ * libfdt - Flat Device Tree manipulation
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ * Copyright 2012 Kim Phillips, Freescale Semiconductor.
+ * Adaptation to QEMU: Copyright IBM Corp. 2008
+ * by Hollis Blanchard holl...@us.ibm.com
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2, as
  * published by the Free Software Foundation.
@@ -11,16 +19,23 @@
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, see http://www.gnu.org/licenses/.
  *
- * Copyright IBM Corp. 2008
- * Authors: Hollis Blanchard holl...@us.ibm.com
  *
  */
 
-#ifndef _LIBFDT_ENV_H
-#define _LIBFDT_ENV_H
-
 #include qemu/bswap.h
 
+#ifdef __CHECKER__
+#define __force __attribute__((force))
+#define __bitwise __attribute__((bitwise))
+#else
+#define __force
+#define __bitwise
+#endif
+
+typedef uint16_t __bitwise fdt16_t;
+typedef uint32_t __bitwise fdt32_t;
+typedef uint64_t __bitwise fdt64_t;
+
 #ifdef HOST_WORDS_BIGENDIAN
 #define fdt32_to_cpu(x)  (x)
 #define cpu_to_fdt32(x)  (x)
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH] fdt: update embedded header file from upstream to fix compilation

[Andre Przywara]

On 05/07/2013 02:44 PM, Peter Maydell wrote:

On 7 May 2013 13:36, Andre Przywara andre.przyw...@linaro.org wrote:

Upstream dtc.git introduced a change in libfdt_env.h, which breaks
compilation with QEMU's version of it:

   CC arm-softmmu/device_tree.o
In file included from /usr/include/libfdt.h:55:0,
  from /src/qemu.git/device_tree.c:28:
/usr/include/fdt.h:7:2: error: unknown type name 'fdt32_t'
...


I'm not entirely sure I understand why we need change.
Have upstream really introduced a breaking change for
everybody who uses libfdt, or are we using it wrongly?


Everybody who copies and changes a header from the original distribution 
and uses this now instead of the installed version ;-)


libfdt (or better: dtc) seems to be quite out of date in the official 
distribution packages. I just happened to have git HEAD installed manually.


Regards,
Andre.

Re: [Qemu-devel] [PATCH] fdt: update embedded header file from upstream to fix compilation

[Andre Przywara]

On 05/07/2013 03:24 PM, Peter Maydell wrote:

On 7 May 2013 13:52, Andre Przywara andre.przyw...@linaro.org wrote:

On 05/07/2013 02:44 PM, Peter Maydell wrote:

I'm not entirely sure I understand why we need change.
Have upstream really introduced a breaking change for
everybody who uses libfdt, or are we using it wrongly?



Everybody who copies and changes a header from the original distribution and
uses this now instead of the installed version ;-)


Right, but the distro libfdt-dev package doesn't ship with
libfdt_env.h, so presumably the expectation is that users
of libfdt copy and adjust it, and so the set of things it
has to provide is part of the public interface?


Seems to be true for the CentOS package for instance, but not for Wheezy:
http://packages.debian.org/wheezy/amd64/libfdt-dev/filelist

Also make install on the source distribution copies the file to 
/usr/include. Even if I delete it, fdt.h still references fdt32_t, so I 
get the same error again.


Regards,
Andre.

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 03/11] arm/arm64: smp: support more than 8 cpus

[Andre Przywara]

Hi,

On 09/11/16 11:57, Andrew Jones wrote:
> On Wed, Nov 09, 2016 at 11:12:03AM +0000, Andre Przywara wrote:
> [...]
>>> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
>>> index 7e7b39f11dde..b6e2d5815e72 100644
>>> --- a/lib/arm/setup.c
>>> +++ b/lib/arm/setup.c
>>> @@ -24,12 +24,22 @@ extern unsigned long stacktop;
>>>  extern void io_init(void);
>>>  extern void setup_args_progname(const char *args);
>>>  
>>> -u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
>>> +u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
>>
>> This should be ~0UL.
> 
> Indeed. Thanks.
> 
>> Also I think the type should be unsigned long to match the types used
>> everywhere else.
> 
> I'll change mpidr_to_cpu to return u64 instead of unsigned long.

I am not sure this is the right direction. unsigned long is really the
natural type for MPIDR, since this is a system register with exactly the
native register size.
I think we use it this way in the kernel.

Cheers,
Andre

> Actually I think Alex suggested that. I'm not sure why I haven't
> done it...
> 
>>
>>>  int nr_cpus;
>>>  
>>>  struct mem_region mem_regions[NR_MEM_REGIONS];
>>>  phys_addr_t __phys_offset, __phys_end;
>>>  
>>> +int mpidr_to_cpu(unsigned long mpidr)
>>> +{
>>> +   int i;
>>> +
>>> +   for (i = 0; i < nr_cpus; ++i)
>>> +   if (cpus[i] == (mpidr & MPIDR_HWID_BITMASK))
>>> +   return i;
>>> +   return -1;
>>> +}
>>> +
>>>  static void cpu_set(int fdtnode __unused, u32 regval, void *info __unused)
>>
>> I guess this needs to be extended as well, including
>> dt_for_each_cpu_node() to cope with 64-bit reg properties in the CPU
>> node (where the upper word is not 0).
>> But this is not really crucial atm, so can be fixed in a follow-up patch.
> 
> Yeah, I'll do it as a followup series, because it'll affect powerpc too.
> 
> drew
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 09/11] arm/arm64: add initial gicv3 support

[Andre Przywara]

Hi,

On 08/11/16 20:21, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> 
> ---
> v4:
>  - only take defines from kernel we need now [Andre]
>  - simplify enable by not caring if we reinit the distributor [drew]
> v2:
>  - configure irqs as NS GRP1
> ---
>  lib/arm/asm/arch_gicv3.h   | 42 +
>  lib/arm/asm/gic-v3.h   | 92 
> ++
>  lib/arm/asm/gic.h  |  1 +
>  lib/arm/gic.c  | 56 
>  lib/arm64/asm/arch_gicv3.h | 44 ++
>  lib/arm64/asm/gic-v3.h |  1 +
>  lib/arm64/asm/sysreg.h | 44 ++
>  7 files changed, 280 insertions(+)
>  create mode 100644 lib/arm/asm/arch_gicv3.h
>  create mode 100644 lib/arm/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/arch_gicv3.h
>  create mode 100644 lib/arm64/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/sysreg.h
> 
> diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
> new file mode 100644
> index ..81a1e5f6c29c
> --- /dev/null
> +++ b/lib/arm/asm/arch_gicv3.h
> @@ -0,0 +1,42 @@
> +/*
> + * All ripped off from arch/arm/include/asm/arch_gicv3.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_ARCH_GICV3_H_
> +#define _ASMARM_ARCH_GICV3_H_
> +
> +#ifndef __ASSEMBLY__
> +#include 
> +#include 
> +#include 
> +
> +#define __stringify xstr
> +
> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2)p15, Op1, %0, CRn, CRm, Op2
> +
> +#define ICC_PMR  __ACCESS_CP15(c4, 0, c6, 0)
> +#define ICC_IGRPEN1  __ACCESS_CP15(c12, 0, c12, 7)
> +
> +static inline void gicv3_write_pmr(u32 val)
> +{
> + asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val));
> +}
> +
> +static inline void gicv3_write_grpen1(u32 val)
> +{
> + asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val));
> + isb();
> +}
> +
> +static inline u64 gicv3_read_typer(const volatile void __iomem *addr)
> +{
> + u64 val = readl(addr);
> + val |= (u64)readl(addr + 4) << 32;
> + return val;
> +}
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* _ASMARM_ARCH_GICV3_H_ */
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> new file mode 100644
> index ..03321f8c860f
> --- /dev/null
> +++ b/lib/arm/asm/gic-v3.h
> @@ -0,0 +1,92 @@
> +/*
> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_V3_H_
> +#define _ASMARM_GIC_V3_H_
> +
> +#ifndef _ASMARM_GIC_H_
> +#error Do not directly include . Include 
> +#endif
> +
> +#define GICD_CTLR0x
> +#define GICD_TYPER   0x0004

So if we share the distributor register definition with GICv2, these
shouldn't be here, but in gic.h.
But this is the right naming scheme we should use (instead of GIC_DIST_xxx).

Now this gets interesting with your wish to both share the definitions
for the GICv2 and GICv3 distributors, but also stick to the names the
kernel uses (because they differ between the two) ;-)
So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER,
for instance.

> +#define GICD_IGROUPR 0x0080
> +
> +#define GICD_CTLR_RWP(1U << 31)
> +#define GICD_CTLR_ARE_NS (1U << 4)
> +#define GICD_CTLR_ENABLE_G1A (1U << 1)
> +#define GICD_CTLR_ENABLE_G1  (1U << 0)
> +
> +#define GICR_TYPER   0x0008
> +#define GICR_IGROUPR0GICD_IGROUPR
> +#define GICR_TYPER_LAST  (1U << 4)
> +
> +
> +#include 
> +
> +#ifndef __ASSEMBLY__
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct gicv3_data {
> + void *dist_base;
> + void *redist_base[NR_CPUS];
> + unsigned int irq_nr;
> +};
> +extern struct gicv3_data gicv3_data;
> +
> +#define gicv3_dist_base()(gicv3_data.dist_base)
> +#define gicv3_redist_base()  
> (gicv3_data.redist_base[smp_processor_id()])
> +#define gicv3_sgi_base() 
> (gicv3_data.redist_base[smp_processor_id()] + SZ_64K)
> +
> +extern int gicv3_init(void);
> +extern void gicv3_enable_defaults(void);
> +
> +static inline void gicv3_do_wait_for_rwp(void *base)
> +{
> + int count = 10; /* 1s */
> +
> + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) {
> + if (!--count) {
> + printf("GICv3: RWP timeout!\n");
> + abort();
> + }
> + cpu_relax();
> + udelay(10);
> + };
> +}
> +
> +static inline void gicv3_dist_wait_for_rwp(void)
> +{
> + gicv3_do_wait_for_rwp(gicv3_dist_base());
> +}
> +
> +static inline void 

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 06/11] arm/arm64: add initial gicv2 support

[Andre Przywara]

Hi,

On 08/11/16 20:21, Andrew Jones wrote:
> Add some gicv2 support. This just adds init and enable
> functions, allowing unit tests to start messing with it.
> 
> Signed-off-by: Andrew Jones 
> 
> ---
> v4:
>  - only take defines from kernel we need now [Andre]
>  - moved defines to asm/gic.h so they'll be shared with v3 [drew]
>  - simplify enable by not caring if we reinit the distributor [drew]
>  - init all GICD_INT_DEF_PRI_X4 registers [Eric]
> ---
>  arm/Makefile.common|  1 +
>  lib/arm/asm/gic-v2.h   | 28 +++
>  lib/arm/asm/gic.h  | 44 +
>  lib/arm/gic.c  | 75 
> ++
>  lib/arm64/asm/gic-v2.h |  1 +
>  lib/arm64/asm/gic.h|  1 +
>  6 files changed, 150 insertions(+)
>  create mode 100644 lib/arm/asm/gic-v2.h
>  create mode 100644 lib/arm/asm/gic.h
>  create mode 100644 lib/arm/gic.c
>  create mode 100644 lib/arm64/asm/gic-v2.h
>  create mode 100644 lib/arm64/asm/gic.h
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index ccb554d9251a..41239c37e092 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -42,6 +42,7 @@ cflatobjs += lib/arm/mmu.o
>  cflatobjs += lib/arm/bitops.o
>  cflatobjs += lib/arm/psci.o
>  cflatobjs += lib/arm/smp.o
> +cflatobjs += lib/arm/gic.o
>  
>  libeabi = lib/arm/libeabi.a
>  eabiobjs = lib/arm/eabi_compat.o
> diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
> new file mode 100644
> index ..f91530f88355
> --- /dev/null
> +++ b/lib/arm/asm/gic-v2.h
> @@ -0,0 +1,28 @@
> +/*
> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_V2_H_
> +#define _ASMARM_GIC_V2_H_
> +
> +#ifndef _ASMARM_GIC_H_
> +#error Do not directly include . Include 
> +#endif
> +
> +struct gicv2_data {
> + void *dist_base;
> + void *cpu_base;
> + unsigned int irq_nr;
> +};
> +extern struct gicv2_data gicv2_data;
> +
> +#define gicv2_dist_base()(gicv2_data.dist_base)
> +#define gicv2_cpu_base() (gicv2_data.cpu_base)
> +
> +extern int gicv2_init(void);
> +extern void gicv2_enable_defaults(void);
> +
> +#endif /* _ASMARM_GIC_V2_H_ */
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> new file mode 100644
> index ..ec92f1064dc0
> --- /dev/null
> +++ b/lib/arm/asm/gic.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_H_
> +#define _ASMARM_GIC_H_
> +
> +#include 
> +
> +#define GIC_CPU_CTRL 0x00
> +#define GIC_CPU_PRIMASK  0x04
> +
> +#define GICC_ENABLE  0x1
> +#define GICC_INT_PRI_THRESHOLD   0xf0
> +
> +#define GIC_DIST_CTRL0x000
> +#define GIC_DIST_CTR 0x004

I think we shouldn't copy this old name here, which stems from pre-GICv2
times (PL390?), IIUC. Both GIC specs talk of TYPER here.

Also if we now use the same defines for both the GICv2 and GICv3
distributor, we should really stick with the (modern) spec naming, which
is GICD_CTRL, GICD_TYPER and so on. Same for the CPU interface (GICC_CTRL).
In the kernel these names are used for GICv3, but we didn't bother to
adjust the existing GICv2 names to avoid pointless churn.
Also that would line up with the bit field defines below.

Cheers,
Andre.

> +#define GIC_DIST_ENABLE_SET  0x100
> +#define GIC_DIST_PRI 0x400
> +
> +#define GICD_ENABLE  0x1
> +#define GICD_INT_EN_SET_SGI  0x
> +#define GICD_INT_DEF_PRI 0xa0
> +#define GICD_INT_DEF_PRI_X4  ((GICD_INT_DEF_PRI << 24) |\
> + (GICD_INT_DEF_PRI << 16) |\
> + (GICD_INT_DEF_PRI << 8) |\
> + GICD_INT_DEF_PRI)
> +
> +#define GICD_TYPER_IRQS(typer)   typer) & 0x1f) + 1) * 32)
> +
> +#ifndef __ASSEMBLY__
> +
> +/*
> + * gic_init will try to find all known gics, and then
> + * initialize the gic data for the one found.
> + * returns
> + *  0   : no gic was found
> + *  > 0 : the gic version of the gic found
> + */
> +extern int gic_init(void);
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* _ASMARM_GIC_H_ */
> diff --git a/lib/arm/gic.c b/lib/arm/gic.c
> new file mode 100644
> index ..91d78c9a0cc2
> --- /dev/null
> +++ b/lib/arm/gic.c
> @@ -0,0 +1,75 @@
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include 
> +#include 
> +#include 
> +
> +struct gicv2_data gicv2_data;
> +
> +/*
> + * 

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 09/11] arm/arm64: add initial gicv3 support

[Andre Przywara]

Hi,

On 09/11/16 13:08, Andrew Jones wrote:
> On Wed, Nov 09, 2016 at 12:35:48PM +0000, Andre Przywara wrote:
> [...]
>>> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
>>> new file mode 100644
>>> index ..03321f8c860f
>>> --- /dev/null
>>> +++ b/lib/arm/asm/gic-v3.h
>>> @@ -0,0 +1,92 @@
>>> +/*
>>> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h
>>> + *
>>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjo...@redhat.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU LGPL, version 2.
>>> + */
>>> +#ifndef _ASMARM_GIC_V3_H_
>>> +#define _ASMARM_GIC_V3_H_
>>> +
>>> +#ifndef _ASMARM_GIC_H_
>>> +#error Do not directly include . Include 
>>> +#endif
>>> +
>>> +#define GICD_CTLR  0x
>>> +#define GICD_TYPER 0x0004
>>
>> So if we share the distributor register definition with GICv2, these
>> shouldn't be here, but in gic.h.
>> But this is the right naming scheme we should use (instead of GIC_DIST_xxx).
>>
>> Now this gets interesting with your wish to both share the definitions
>> for the GICv2 and GICv3 distributors, but also stick to the names the
>> kernel uses (because they differ between the two) ;-)
>> So now you loose the greppability for either GIC_DIST_CTR or GICD_TYPER,
>> for instance.
> 
> Well, we just have the same offset with two names (giving us two
> symbols to grep). I put them here, vs. asm/gic.h, because the kernel
> only uses theses symbols for gicv3. Now, nothing stops a unit test
> from using them with gicv2 tests, though, because unit tests include
> gic.h, which includes both gic-v2.h and gic-v3.h, and thus it gets
> both. I know, it's sounding messy... Shouldn't we post some churn to
> the kernel? :-)

Well, on top of that the distributor registers are slightly different
(check CTLR and TYPER, for instance). So it's churn plus a stretch, I
guess Marc won't like that.

So if greppability is important, should we revert to separate
definitions in separate header files then, like in v3?
I don't think we actually share _code_ between the two GIC revisions, do we?

> Note, I tried to only add defines to asm/gic.h that are actually
> shared in the kernel between v2 and v3, e.g. GIC_DIST_ENABLE_SET.

Huh? GICv3 uses GICD_ISENABLER for that register.

> Actually, GIC_DIST_CTRL and GIC_DIST_CTR may be the only exceptions
> we have so far.

Note that it's GIC_DIST_CTLR (L and R swapped), one reason more to dump
_CTR ;-)

>>
>>> +#define GICD_IGROUPR   0x0080
>>> +
>>> +#define GICD_CTLR_RWP  (1U << 31)
>>> +#define GICD_CTLR_ARE_NS   (1U << 4)
>>> +#define GICD_CTLR_ENABLE_G1A   (1U << 1)
>>> +#define GICD_CTLR_ENABLE_G1(1U << 0)
>>> +
>>> +#define GICR_TYPER 0x0008
>>> +#define GICR_IGROUPR0  GICD_IGROUPR
>>> +#define GICR_TYPER_LAST(1U << 4)
>>> +
>>> +
>>> +#include 
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +struct gicv3_data {
>>> +   void *dist_base;
>>> +   void *redist_base[NR_CPUS];
>>> +   unsigned int irq_nr;
>>> +};
>>> +extern struct gicv3_data gicv3_data;
>>> +
>>> +#define gicv3_dist_base()  (gicv3_data.dist_base)
>>> +#define gicv3_redist_base()
>>> (gicv3_data.redist_base[smp_processor_id()])
>>> +#define gicv3_sgi_base()   
>>> (gicv3_data.redist_base[smp_processor_id()] + SZ_64K)
>>> +
>>> +extern int gicv3_init(void);
>>> +extern void gicv3_enable_defaults(void);
>>> +
>>> +static inline void gicv3_do_wait_for_rwp(void *base)
>>> +{
>>> +   int count = 10; /* 1s */
>>> +
>>> +   while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) {
>>> +   if (!--count) {
>>> +   printf("GICv3: RWP timeout!\n");
>>> +   abort();
>>> +   }
>>> +   cpu_relax();
>>> +   udelay(10);
>>> +   };
>>> +}
>>> +
>>> +static inline void gicv3_dist_wait_for_rwp(void)
>>> +{
>>> +   gicv3_do_wait_for_rwp(gicv3_dist_base());
>>> +}
>>> +
>>> +static inline void gicv3_redist_

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 08/11] libcflat: add IS_ALIGNED() macro, and page sizes

[Andre Przywara]

Hi,

On 08/11/16 20:21, Andrew Jones wrote:
> From: Peter Xu <pet...@redhat.com>
> 
> These macros will be useful to do page alignment checks.

Reviewed-by: Andre Przywara <andre.przyw...@arm.com>

Cheers,
Andre.

> Signed-off-by: Peter Xu <pet...@redhat.com>
> [drew: also added SZ_64K]
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
> ---
>  lib/libcflat.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 82005f5d014f..143fc53061fe 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -33,6 +33,12 @@
>  #define __ALIGN_MASK(x, mask)(((x) + (mask)) & ~(mask))
>  #define __ALIGN(x, a)__ALIGN_MASK(x, (typeof(x))(a) - 1)
>  #define ALIGN(x, a)  __ALIGN((x), (a))
> +#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
> +
> +#define SZ_4K(0x1000)
> +#define SZ_64K   (0x1)
> +#define SZ_2M(0x20)
> +#define SZ_1G(0x4000)
>  
>  typedef uint8_t  u8;
>  typedef int8_t   s8;
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 03/11] arm/arm64: smp: support more than 8 cpus

[Andre Przywara]

Hi,

On 08/11/16 20:21, Andrew Jones wrote:
> By adding support for launching with gicv3 we can break the 8 vcpu
> limit. This patch adds support to smp code and also selects the
> vgic model corresponding to the host. The vgic model may also be
> manually selected by adding e.g. -machine gic-version=3 to
> extra_params.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Andrew Jones 
> 
> ---
> v4: improved commit message
> ---
>  arm/run   | 19 ---
>  arm/selftest.c|  5 -
>  lib/arm/asm/processor.h   |  9 +++--
>  lib/arm/asm/setup.h   |  4 ++--
>  lib/arm/setup.c   | 12 +++-
>  lib/arm64/asm/processor.h |  9 +++--
>  6 files changed, 43 insertions(+), 15 deletions(-)
> 
> diff --git a/arm/run b/arm/run
> index a2f35ef6a7e6..2d0698619606 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -31,13 +31,6 @@ if [ -z "$ACCEL" ]; then
>   fi
>  fi
>  
> -if [ "$HOST" = "aarch64" ] && [ "$ACCEL" = "kvm" ]; then
> - processor="host"
> - if [ "$ARCH" = "arm" ]; then
> - processor+=",aarch64=off"
> - fi
> -fi
> -
>  qemu="${QEMU:-qemu-system-$ARCH_NAME}"
>  qpath=$(which $qemu 2>/dev/null)
>  
> @@ -53,6 +46,18 @@ fi
>  
>  M='-machine virt'
>  
> +if [ "$ACCEL" = "kvm" ]; then
> + if $qemu $M,\? 2>&1 | grep gic-version > /dev/null; then
> + M+=',gic-version=host'
> + fi
> + if [ "$HOST" = "aarch64" ]; then
> + processor="host"
> + if [ "$ARCH" = "arm" ]; then
> + processor+=",aarch64=off"
> + fi
> + fi
> +fi
> +
>  if ! $qemu $M -device '?' 2>&1 | grep virtconsole > /dev/null; then
>   echo "$qpath doesn't support virtio-console for chr-testdev. Exiting."
>   exit 2
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 196164f5313d..2f117f795d2d 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -312,9 +312,10 @@ static bool psci_check(void)
>  static cpumask_t smp_reported;
>  static void cpu_report(void)
>  {
> + unsigned long mpidr = get_mpidr();
>   int cpu = smp_processor_id();
>  
> - report("CPU%d online", true, cpu);
> + report("CPU(%3d) mpidr=%lx", mpidr_to_cpu(mpidr) == cpu, cpu, mpidr);
>   cpumask_set_cpu(cpu, _reported);
>   halt();
>  }
> @@ -343,6 +344,7 @@ int main(int argc, char **argv)
>  
>   } else if (strcmp(argv[1], "smp") == 0) {
>  
> + unsigned long mpidr = get_mpidr();
>   int cpu;
>  
>   report("PSCI version", psci_check());
> @@ -353,6 +355,7 @@ int main(int argc, char **argv)
>   smp_boot_secondary(cpu, cpu_report);
>   }
>  
> + report("CPU(%3d) mpidr=%lx", mpidr_to_cpu(mpidr) == 0, 0, 
> mpidr);
>   cpumask_set_cpu(0, _reported);
>   while (!cpumask_full(_reported))
>   cpu_relax();
> diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
> index f25e7eee3666..d2048f5f5f7e 100644
> --- a/lib/arm/asm/processor.h
> +++ b/lib/arm/asm/processor.h
> @@ -40,8 +40,13 @@ static inline unsigned int get_mpidr(void)
>   return mpidr;
>  }
>  
> -/* Only support Aff0 for now, up to 4 cpus */
> -#define mpidr_to_cpu(mpidr) ((int)((mpidr) & 0xff))
> +#define MPIDR_HWID_BITMASK 0xff
> +extern int mpidr_to_cpu(unsigned long mpidr);
> +
> +#define MPIDR_LEVEL_SHIFT(level) \
> + (((1 << level) >> 1) << 3)
> +#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
> + ((mpidr >> MPIDR_LEVEL_SHIFT(level)) & 0xff)
>  
>  extern void start_usr(void (*func)(void *arg), void *arg, unsigned long 
> sp_usr);
>  extern bool is_user(void);
> diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> index cb8fdbd38dd5..b0d51f5f0721 100644
> --- a/lib/arm/asm/setup.h
> +++ b/lib/arm/asm/setup.h
> @@ -10,8 +10,8 @@
>  #include 
>  #include 
>  
> -#define NR_CPUS  8
> -extern u32 cpus[NR_CPUS];
> +#define NR_CPUS  255
> +extern u64 cpus[NR_CPUS];/* per-cpu IDs (MPIDRs) */
>  extern int nr_cpus;
>  
>  #define NR_MEM_REGIONS   8
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 7e7b39f11dde..b6e2d5815e72 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -24,12 +24,22 @@ extern unsigned long stacktop;
>  extern void io_init(void);
>  extern void setup_args_progname(const char *args);
>  
> -u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> +u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };

This should be ~0UL.
Also I think the type should be unsigned long to match the types used
everywhere else.

>  int nr_cpus;
>  
>  struct mem_region mem_regions[NR_MEM_REGIONS];
>  phys_addr_t __phys_offset, __phys_end;
>  
> +int mpidr_to_cpu(unsigned long mpidr)
> +{
> + int i;
> +
> + for (i = 0; i < nr_cpus; ++i)
> + if (cpus[i] == (mpidr & MPIDR_HWID_BITMASK))
> + return i;
> + return -1;
> +}
> +
>  static 

[Qemu-devel] [RFC PATCH] kvm-unit-tests: arm/arm64: strip GIC headers

[Andre Przywara]

Hi,

this is an illustrative patch which shows what can be removed from
the kvm-unit-tests GIC headers. If this patch finds mercy, it should be
squashed into the in-flight GIC patches, eventually.
The rationale for this patch is that while it seems like a good idea to
copy the header files with the GIC definitions from Linux, we should
avoid the danger of copying bugs on the way. This is especially true
for those definitions that are used by the very emulation code that we
are about to test. Beside this the headers contain much more definitions
than we actually need.
So strip those header files, by first removing all definitions that are
actually only useful for hypervisors. Also we remove definitions for
registers that are not needed yet. The idea is to add them one by one as
soon as we start to test functionality, so that review can be much easier
and safer.
Applies on top of Drew's v3 GIC series (as in his github branch).

Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
---
 lib/arm/asm/arch_gicv3.h   |  62 -
 lib/arm/asm/gic-v2.h   |  22 -
 lib/arm/asm/gic-v3.h   | 227 -
 lib/arm64/asm/arch_gicv3.h |  43 -
 4 files changed, 354 deletions(-)

diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
index d529a7e..333bf49 100644
--- a/lib/arm/asm/arch_gicv3.h
+++ b/lib/arm/asm/arch_gicv3.h
@@ -16,7 +16,6 @@
 
 #define __stringify xstr
 
-
 #define __ACCESS_CP15(CRn, Op1, CRm, Op2)  p15, Op1, %0, CRn, CRm, Op2
 #define __ACCESS_CP15_64(Op1, CRm) p15, Op1, %Q0, %R0, CRm
 
@@ -31,67 +30,6 @@
 
 #define ICC_HSRE   __ACCESS_CP15(c12, 4, c9, 5)
 
-#define ICH_VSEIR  __ACCESS_CP15(c12, 4, c9, 4)
-#define ICH_HCR__ACCESS_CP15(c12, 4, c11, 0)
-#define ICH_VTR__ACCESS_CP15(c12, 4, c11, 1)
-#define ICH_MISR   __ACCESS_CP15(c12, 4, c11, 2)
-#define ICH_EISR   __ACCESS_CP15(c12, 4, c11, 3)
-#define ICH_ELSR   __ACCESS_CP15(c12, 4, c11, 5)
-#define ICH_VMCR   __ACCESS_CP15(c12, 4, c11, 7)
-
-#define __LR0(x)   __ACCESS_CP15(c12, 4, c12, x)
-#define __LR8(x)   __ACCESS_CP15(c12, 4, c13, x)
-
-#define ICH_LR0__LR0(0)
-#define ICH_LR1__LR0(1)
-#define ICH_LR2__LR0(2)
-#define ICH_LR3__LR0(3)
-#define ICH_LR4__LR0(4)
-#define ICH_LR5__LR0(5)
-#define ICH_LR6__LR0(6)
-#define ICH_LR7__LR0(7)
-#define ICH_LR8__LR8(0)
-#define ICH_LR9__LR8(1)
-#define ICH_LR10   __LR8(2)
-#define ICH_LR11   __LR8(3)
-#define ICH_LR12   __LR8(4)
-#define ICH_LR13   __LR8(5)
-#define ICH_LR14   __LR8(6)
-#define ICH_LR15   __LR8(7)
-
-/* LR top half */
-#define __LRC0(x)  __ACCESS_CP15(c12, 4, c14, x)
-#define __LRC8(x)  __ACCESS_CP15(c12, 4, c15, x)
-
-#define ICH_LRC0   __LRC0(0)
-#define ICH_LRC1   __LRC0(1)
-#define ICH_LRC2   __LRC0(2)
-#define ICH_LRC3   __LRC0(3)
-#define ICH_LRC4   __LRC0(4)
-#define ICH_LRC5   __LRC0(5)
-#define ICH_LRC6   __LRC0(6)
-#define ICH_LRC7   __LRC0(7)
-#define ICH_LRC8   __LRC8(0)
-#define ICH_LRC9   __LRC8(1)
-#define ICH_LRC10  __LRC8(2)
-#define ICH_LRC11  __LRC8(3)
-#define ICH_LRC12  __LRC8(4)
-#define ICH_LRC13  __LRC8(5)
-#define ICH_LRC14  __LRC8(6)
-#define ICH_LRC15  __LRC8(7)
-
-#define __AP0Rx(x) __ACCESS_CP15(c12, 4, c8, x)
-#define ICH_AP0R0  __AP0Rx(0)
-#define ICH_AP0R1  __AP0Rx(1)
-#define ICH_AP0R2  __AP0Rx(2)
-#define ICH_AP0R3  __AP0Rx(3)
-
-#define __AP1Rx(x) __ACCESS_CP15(c12, 4, c9, x)
-#define ICH_AP1R0  __AP1Rx(0)
-#define ICH_AP1R1  __AP1Rx(1)
-#define ICH_AP1R2  __AP1Rx(2)
-#define ICH_AP1R3  __AP1Rx(3)
-
 /* Low-level accessors */
 
 static inline void gicv3_write_eoir(u32 irq)
diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
index 973c2bf..019d553 100644
--- a/lib/arm/asm/gic-v2.h
+++ b/lib/arm/asm/gic-v2.h
@@ -10,44 +10,22 @@
 
 #define GIC_CP

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 10/11] arm/arm64: gicv3: add an IPI test

[Andre Przywara]

Hi,

On 10/11/16 19:53, Alex Bennée wrote:


> So I was re-basing my kvm-unit-tests against your GIC rework and found
> myself copy and pasting a bunch of this into my tests that fire IRQs.

So I take it you are working on (or already have) code to test SPIs,
probably via GICD_ISPENDR?
Just asking because I was thinking about going there and thus could save
my time if you are on it already...

> That makes me think the abstraction should be in the library code so
> other tests can fiddle with sending IRQs.

...because I was wondering the same.

Cheers,
Andre.

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 07/11] arm/arm64: gicv2: add an IPI test

[Andre Przywara]

Hi,

more a comment loosely related to this patch ...

> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 3f6fa45c587e..68bf5cd6008f 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -54,3 +54,10 @@ file = selftest.flat
>  smp = $MAX_SMP
>  extra_params = -append 'smp'
>  groups = selftest
> +
> +# Test GIC emulation
> +[gicv2-ipi]
> +file = gic.flat
> +smp = $((($MAX_SMP < 8)?$MAX_SMP:8))

So here we always go with the maximum number of VCPUs in the guest.
However (as you also noted in your cover-letter) running with a
different number of CPUs might be interesting, for instance with less
than 8 CPUs on a GICv2 (the ITARGETSR register must be masked) or in
general with an odd number (both literally and in the broader sense). I
have a test case with passes with 8 VCPUs but fails with less.

Is there any good way to run some tests multiple times with different
numbers of VCPUS?
Shall we add some "set" functionality to the smp parameter, so that we
can specify a list of desired test points?

Cheers,
Andre.

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 06/11] arm/arm64: add initial gicv2 support

[Andre Przywara]

Hi,

On 11/11/16 14:52, Alex Bennée wrote:
> 
> Andrew Jones  writes:
> 
>> Add some gicv2 support. This just adds init and enable
>> functions, allowing unit tests to start messing with it.
>>
>> Signed-off-by: Andrew Jones 
>>
>> ---
>> v5: share/use only the modern register names [Andre]
>> v4:
>>  - only take defines from kernel we need now [Andre]
>>  - moved defines to asm/gic.h so they'll be shared with v3 [drew]
>>  - simplify enable by not caring if we reinit the distributor [drew]
>>  - init all GICD_INT_DEF_PRI_X4 registers [Eric]
>> ---
>>  arm/Makefile.common|  1 +
>>  lib/arm/asm/gic-v2.h   | 34 ++
>>  lib/arm/asm/gic.h  | 37 
>>  lib/arm/gic.c  | 76 
>> ++
>>  lib/arm64/asm/gic-v2.h |  1 +
>>  lib/arm64/asm/gic.h|  1 +
>>  6 files changed, 150 insertions(+)
>>  create mode 100644 lib/arm/asm/gic-v2.h
>>  create mode 100644 lib/arm/asm/gic.h
>>  create mode 100644 lib/arm/gic.c
>>  create mode 100644 lib/arm64/asm/gic-v2.h
>>  create mode 100644 lib/arm64/asm/gic.h
>>
>> diff --git a/arm/Makefile.common b/arm/Makefile.common
>> index ccb554d9251a..41239c37e092 100644
>> --- a/arm/Makefile.common
>> +++ b/arm/Makefile.common
>> @@ -42,6 +42,7 @@ cflatobjs += lib/arm/mmu.o
>>  cflatobjs += lib/arm/bitops.o
>>  cflatobjs += lib/arm/psci.o
>>  cflatobjs += lib/arm/smp.o
>> +cflatobjs += lib/arm/gic.o
>>
>>  libeabi = lib/arm/libeabi.a
>>  eabiobjs = lib/arm/eabi_compat.o
>> diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
>> new file mode 100644
>> index ..c2d5fecd4886
>> --- /dev/null
>> +++ b/lib/arm/asm/gic-v2.h
>> @@ -0,0 +1,34 @@
>> +/*
>> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic.h
>> + *
>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.
>> + */
>> +#ifndef _ASMARM_GIC_V2_H_
>> +#define _ASMARM_GIC_V2_H_
>> +
>> +#ifndef _ASMARM_GIC_H_
>> +#error Do not directly include . Include 
>> +#endif
>> +
>> +#define GICD_ENABLE 0x1
>> +#define GICC_ENABLE 0x1
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +struct gicv2_data {
>> +void *dist_base;
>> +void *cpu_base;
>> +unsigned int irq_nr;
>> +};
>> +extern struct gicv2_data gicv2_data;
>> +
>> +#define gicv2_dist_base()   (gicv2_data.dist_base)
>> +#define gicv2_cpu_base()(gicv2_data.cpu_base)
>> +
>> +extern int gicv2_init(void);
>> +extern void gicv2_enable_defaults(void);
>> +
>> +#endif /* !__ASSEMBLY__ */
>> +#endif /* _ASMARM_GIC_V2_H_ */
>> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
>> new file mode 100644
>> index ..d44e47bcf404
>> --- /dev/null
>> +++ b/lib/arm/asm/gic.h
>> @@ -0,0 +1,37 @@
>> +/*
>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.
>> + */
>> +#ifndef _ASMARM_GIC_H_
>> +#define _ASMARM_GIC_H_
>> +
>> +#include 
>> +
>> +#define GICD_CTLR   0x
>> +#define GICD_TYPER  0x0004
>> +#define GICD_ISENABLER  0x0100
>> +#define GICD_IPRIORITYR 0x0400
> 
> Maybe GICD_ISENABLER_BASE and GICD_IPRIORITYR_BASE as they are the start
> of a series of registers?

We should keep the naming consistent to both the spec and the Linux headers.

> Also what happened to the formatting?

Isn't that the usual diff artifact caused by prepending a single
character? Which moves the tab stops, also the mailer adding quotation
characters?

>> +
>> +#define GICD_TYPER_IRQS(typer)  typer) & 0x1f) + 1)
>> * 32)
>> +#define GICD_INT_EN_SET_SGI 0x
>> +#define GICD_INT_DEF_PRI_X4 0xa0a0a0a0
> 
> This doesn't seem to be used and I'm not sure what GICD_TYPER_IRQS it is
> trying to achieve.

The idea is to calculate the number of implemented SPIs. But I am not a
big fan of copying a macro from the emulation code base to the test code.

Cheers,
Andre.

> A comment above and here to make it clear we are talking about offsets
> in the distributor and cpu register maps would aid confusion.
> 
>> +
>> +#define GICC_CTLR   0x
>> +#define GICC_PMR0x0004
>> +
>> +#define GICC_INT_PRI_THRESHOLD  0xf0
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +/*
>> + * gic_init will try to find all known gics, and then
>> + * initialize the gic data for the one found.
>> + * returns
>> + *  0   : no gic was found
>> + *  > 0 : the gic version of the gic found
>> + */
>> +extern int gic_init(void);
> 
> If we are going to make the library API agnostic I guess returning NULL
> or an ops structure would be best here?
> 
>> +
>> +#endif /* !__ASSEMBLY__ */
>> +#endif /* _ASMARM_GIC_H_ */
>> diff --git a/lib/arm/gic.c b/lib/arm/gic.c
>> new file mode 100644
>> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 10/11] arm/arm64: gicv3: add an IPI test

[Andre Przywara]

Hi,

On 11/11/16 14:53, Alex Bennée wrote:
> 
> Andrew Jones  writes:
> 
>> On Fri, Nov 11, 2016 at 10:02:59AM +, Alex Bennée wrote:
>>>
>>> Andrew Jones  writes:
>>>
 On Thu, Nov 10, 2016 at 07:53:58PM +, Alex Bennée wrote:
 [...]
>> +struct gic gicv2 = {
>> +.ipi = {
>> +.enable = gicv2_enable_defaults,
>> +.send_self = gicv2_ipi_send_self,
>> +.send_tlist = gicv2_ipi_send_tlist,
>> +.send_broadcast = gicv2_ipi_send_broadcast,
>> +},
>> +.read_iar = gicv2_read_iar,
>> +.irqnr = gicv2_irqnr,
>> +.write_eoi = gicv2_write_eoi,
>> +};
>> +
>> +struct gic gicv3 = {
>> +.ipi = {
>> +.enable = gicv3_enable_defaults,
>> +.send_self = gicv3_ipi_send_self,
>> +.send_tlist = gicv3_ipi_send_tlist,
>> +.send_broadcast = gicv3_ipi_send_broadcast,
>> +},
>> +.read_iar = gicv3_read_iar,
>> +.irqnr = gicv3_irqnr,
>> +.write_eoi = gicv3_write_eoir,
>> +};
>> +
>
> So I was re-basing my kvm-unit-tests against your GIC rework and found
> myself copy and pasting a bunch of this into my tests that fire IRQs.
> That makes me think the abstraction should be in the library code so
> other tests can fiddle with sending IRQs.
>
> What do you think?
>

 I guess you mean moving the above two structs and their corresponding
 functions (all which aren't already common) to lib/arm/ ? Or do you
 just mean the one non-trivial function gicv3_ipi_send_tlist? I think
 agree with gicv3_ipi_send_tlist getting shared, but the others are
 mostly one-liners, so I'm not sure. I guess I'd have to see how you're
 using them first.
>>>
>>> So it looked like there were some functions in the common code for one
>>> GIC which had local test defined functions for the other. They should at
>>> least be consistent.
>>
>> gicv3_read_iar and gicv3_write_eoir being common already is a product of
>> being sysreg wrappers, allowing for both arm32 and arm64 to use functions
>> of the same names, not because I wanted gicv3 to be inconsistent with
>> gicv2 (which uses MMIO and thus doesn't need wrappers)
>>
>>>
>>> For my use case I could do with a common:
>>>
>>>   gic_enable
>>
>> OK, I can extend gic_init() to initialize a 'struct gic_common_ops' that
>> includes an enable -> *_enable_defaults(void), ipi_send(int cpu),
>> read_iar(void), iar_irqnr(u32 iar), and write_eoi(u32 irqstat). And also
>> provide the wrappers gic_enable, gic_ipi_send(cpu), ...
>>
>>>   gic_send_spi(cpu, irq)
>>
>> I'll let you add this one to the new common ops struct :-)
>>
>>>   gic_irq_ack() which returns the iar.
>>
>> This one will be called read_iar.
>>
>> Would that work for you, Alex?
> 
> Sounds good to me :-)
> 
>>
>> Andre,
>>
>> Would this also satisfy your needs for more common code?

TBH I haven't look deeply enough to give an educated answer. I just
wanted to +1 Alex for the general direction. So I guess it's OK. ;-)

I guess we may need some refactoring later anyway, so any missing pieces
can be added/refactored later once we exactly know what we need.

Cheers,
Andre.

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 03/11] arm/arm64: smp: support more than 8 cpus

[Andre Przywara]

Hi,

On 10/11/16 17:21, Andrew Jones wrote:
> By adding support for launching with gicv3 we can break the 8 vcpu
> limit. This patch adds support to smp code and also selects the
> vgic model corresponding to the host. The vgic model may also be
> manually selected by adding e.g. -machine gic-version=3 to
> extra_params.
> 
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
> 
> ---
> v5: left cpus a u32 for now. Changing to u64 requires a change to
> devicetree. Will do it later. [Andre]

Given that we address this in the future:

Reviewed-by: Andre Przywara <andre.przyw...@arm.com>

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 06/11] arm/arm64: add initial gicv2 support

[Andre Przywara]

Hi,

On 10/11/16 17:21, Andrew Jones wrote:
> Add some gicv2 support. This just adds init and enable
> functions, allowing unit tests to start messing with it.
> 
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
> 
> ---
> v5: share/use only the modern register names [Andre]

Thanks! That looks much better now.

Reviewed-by: Andre Przywara <andre.przyw...@arm.com>

> v4:
>  - only take defines from kernel we need now [Andre]
>  - moved defines to asm/gic.h so they'll be shared with v3 [drew]
>  - simplify enable by not caring if we reinit the distributor [drew]
>  - init all GICD_INT_DEF_PRI_X4 registers [Eric]
> ---
>  arm/Makefile.common|  1 +
>  lib/arm/asm/gic-v2.h   | 34 ++
>  lib/arm/asm/gic.h  | 37 
>  lib/arm/gic.c  | 76 
> ++
>  lib/arm64/asm/gic-v2.h |  1 +
>  lib/arm64/asm/gic.h|  1 +
>  6 files changed, 150 insertions(+)
>  create mode 100644 lib/arm/asm/gic-v2.h
>  create mode 100644 lib/arm/asm/gic.h
>  create mode 100644 lib/arm/gic.c
>  create mode 100644 lib/arm64/asm/gic-v2.h
>  create mode 100644 lib/arm64/asm/gic.h
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index ccb554d9251a..41239c37e092 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -42,6 +42,7 @@ cflatobjs += lib/arm/mmu.o
>  cflatobjs += lib/arm/bitops.o
>  cflatobjs += lib/arm/psci.o
>  cflatobjs += lib/arm/smp.o
> +cflatobjs += lib/arm/gic.o
>  
>  libeabi = lib/arm/libeabi.a
>  eabiobjs = lib/arm/eabi_compat.o
> diff --git a/lib/arm/asm/gic-v2.h b/lib/arm/asm/gic-v2.h
> new file mode 100644
> index ..c2d5fecd4886
> --- /dev/null
> +++ b/lib/arm/asm/gic-v2.h
> @@ -0,0 +1,34 @@
> +/*
> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjo...@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_V2_H_
> +#define _ASMARM_GIC_V2_H_
> +
> +#ifndef _ASMARM_GIC_H_
> +#error Do not directly include . Include 
> +#endif
> +
> +#define GICD_ENABLE  0x1
> +#define GICC_ENABLE  0x1
> +
> +#ifndef __ASSEMBLY__
> +
> +struct gicv2_data {
> + void *dist_base;
> + void *cpu_base;
> + unsigned int irq_nr;
> +};
> +extern struct gicv2_data gicv2_data;
> +
> +#define gicv2_dist_base()(gicv2_data.dist_base)
> +#define gicv2_cpu_base() (gicv2_data.cpu_base)
> +
> +extern int gicv2_init(void);
> +extern void gicv2_enable_defaults(void);
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* _ASMARM_GIC_V2_H_ */
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> new file mode 100644
> index ..d44e47bcf404
> --- /dev/null
> +++ b/lib/arm/asm/gic.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjo...@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_H_
> +#define _ASMARM_GIC_H_
> +
> +#include 
> +
> +#define GICD_CTLR0x
> +#define GICD_TYPER   0x0004
> +#define GICD_ISENABLER   0x0100
> +#define GICD_IPRIORITYR  0x0400
> +
> +#define GICD_TYPER_IRQS(typer)   typer) & 0x1f) + 1) * 32)
> +#define GICD_INT_EN_SET_SGI  0x
> +#define GICD_INT_DEF_PRI_X4  0xa0a0a0a0
> +
> +#define GICC_CTLR0x
> +#define GICC_PMR 0x0004
> +
> +#define GICC_INT_PRI_THRESHOLD   0xf0
> +
> +#ifndef __ASSEMBLY__
> +
> +/*
> + * gic_init will try to find all known gics, and then
> + * initialize the gic data for the one found.
> + * returns
> + *  0   : no gic was found
> + *  > 0 : the gic version of the gic found
> + */
> +extern int gic_init(void);
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* _ASMARM_GIC_H_ */
> diff --git a/lib/arm/gic.c b/lib/arm/gic.c
> new file mode 100644
> index ..d655105e058b
> --- /dev/null
> +++ b/lib/arm/gic.c
> @@ -0,0 +1,76 @@
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjo...@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include 
> +#include 
> +#include 
> +
> +struct gicv2_data gicv2_data;
> +
> +/*
> + * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
> + */
> +static bool
> +gic_get_dt_bases(const char *compatible, void **base

Re: [Qemu-devel] [kvm-unit-tests PATCH v4 00/11] arm/arm64: add gic framework

[Andre Przywara]

Hi,

so is this actually v4 just resent?
Or is this is a new version with s/5/4/?
I can't spot any of the key changes quickly ...

Cheers,
Andre.

On 10/11/16 16:07, Andrew Jones wrote:
> v4:
>  - Eric's r-b's
>  - Andre's suggestion to only take defines we need
>  - several other changes listed in individual patches
> 
> v3:
>  - Rebased on latest master
>  - Added Alex's r-b's
> 
> v2:
>  Rebased on latest master + my "populate argv[0]" series (will
>  send a REPOST for that shortly. Additionally a few patches got
>  fixes/features;
>  07/10 got same fix as kernel 7c9b973061 "irqchip/gic-v3: Configure
>all interrupts as non-secure Group-1" in order to continue
>working over TCG, as the gicv3 code for TCG removed a hack
>it had there to make Linux happy.
>  08/10 added more output for when things fail (if they fail)
>  09/10 switched gicv3 broadcast implementation to using IRM. This
>found a bug in a recent (but not tip) kernel, which I was
>about to fix, but then I saw MarcZ beat me to it.
>  10/10 actually check that the input irq is the received irq
> 
> 
> Import defines, and steal enough helper functions, from Linux to
> enable programming of the gic (v2 and v3). Then use the framework
> to add an initial test (an ipi test; self, target-list, broadcast).
> 
> It's my hope that this framework will be a suitable base on which
> more tests may be easily added, particularly because we have
> vgic-new and tcg gicv3 emulation getting close to merge. (v3 UPDATE:
> vgic-new and tcg gicv3 are merged now)
> 
> To run it, along with other tests, just do
> 
>  ./configure [ --arch=[arm|arm64] --cross-prefix=$PREFIX ]
>  make
>  export QEMU=$PATH_TO_QEMU
>  ./run_tests.sh
> 
> To run it separately do, e.g.
> 
> $QEMU -machine virt,accel=tcg -cpu cortex-a57 \
>  -device virtio-serial-device \
>  -device virtconsole,chardev=ctd -chardev testdev,id=ctd \
>  -display none -serial stdio \
>  -kernel arm/gic.flat \
>  -smp 123 -machine gic-version=3 -append ipi
>   ^^ note, we can go nuts with nr-cpus on TCG :-)
> 
> Or, a KVM example using a different "sender" cpu and irq (other than zero)
> 
> $QEMU -machine virt,accel=kvm -cpu host \
>  -device virtio-serial-device \
>  -device virtconsole,chardev=ctd -chardev testdev,id=ctd \
>  -display none -serial stdio \
>  -kernel arm/gic.flat \
>  -smp 48 -machine gic-version=3 -append 'ipi sender=42 irq=1'
> 
> 
> Patches:
> 01-05: fixes and functionality needed by the later gic patches
> 06-07: enable gicv2 and gicv2 IPI test
> 08-10: enable gicv3 and gicv3 IPI test
>11: extend the IPI tests to take variable sender and irq
> 
> Available here: https://github.com/rhdrjones/kvm-unit-tests/commits/arm/gic-v4
> 
> 
> Andrew Jones (10):
>   lib: xstr: allow multiple args
>   arm64: fix get_"sysreg32" and make MPIDR 64bit
>   arm/arm64: smp: support more than 8 cpus
>   arm/arm64: add some delay routines
>   arm/arm64: irq enable/disable
>   arm/arm64: add initial gicv2 support
>   arm/arm64: gicv2: add an IPI test
>   arm/arm64: add initial gicv3 support
>   arm/arm64: gicv3: add an IPI test
>   arm/arm64: gic: don't just use zero
> 
> Peter Xu (1):
>   libcflat: add IS_ALIGNED() macro, and page sizes
> 
>  arm/Makefile.common|   7 +-
>  arm/gic.c  | 417 
> +
>  arm/run|  19 ++-
>  arm/selftest.c |   5 +-
>  arm/unittests.cfg  |  13 ++
>  lib/arm/asm/arch_gicv3.h   |  65 +++
>  lib/arm/asm/gic-v2.h   |  28 +++
>  lib/arm/asm/gic-v3.h   |  92 ++
>  lib/arm/asm/gic.h  |  51 ++
>  lib/arm/asm/processor.h|  38 -
>  lib/arm/asm/setup.h|   4 +-
>  lib/arm/gic.c  | 131 ++
>  lib/arm/processor.c|  15 ++
>  lib/arm/setup.c|  12 +-
>  lib/arm64/asm/arch_gicv3.h |  66 +++
>  lib/arm64/asm/gic-v2.h |   1 +
>  lib/arm64/asm/gic-v3.h |   1 +
>  lib/arm64/asm/gic.h|   1 +
>  lib/arm64/asm/processor.h  |  53 +-
>  lib/arm64/asm/sysreg.h |  44 +
>  lib/arm64/processor.c  |  15 ++
>  lib/libcflat.h |  10 +-
>  22 files changed, 1062 insertions(+), 26 deletions(-)
>  create mode 100644 arm/gic.c
>  create mode 100644 lib/arm/asm/arch_gicv3.h
>  create mode 100644 lib/arm/asm/gic-v2.h
>  create mode 100644 lib/arm/asm/gic-v3.h
>  create mode 100644 lib/arm/asm/gic.h
>  create mode 100644 lib/arm/gic.c
>  create mode 100644 lib/arm64/asm/arch_gicv3.h
>  create mode 100644 lib/arm64/asm/gic-v2.h
>  create mode 100644 lib/arm64/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/gic.h
>  create mode 100644 lib/arm64/asm/sysreg.h
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH v3 07/10] arm/arm64: add initial gicv3 support

[Andre Przywara]

Hi Drew,

On 15/07/16 14:00, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> 
> ---
> v2: configure irqs as NS GRP1
> ---
>  lib/arm/asm/arch_gicv3.h   | 184 ++
>  lib/arm/asm/gic-v3.h   | 321 
> +
>  lib/arm/asm/gic.h  |   1 +
>  lib/arm/gic.c  |  73 +++
>  lib/arm64/asm/arch_gicv3.h | 169 
>  lib/arm64/asm/gic-v3.h |   1 +
>  lib/arm64/asm/sysreg.h |  44 +++
>  7 files changed, 793 insertions(+)
>  create mode 100644 lib/arm/asm/arch_gicv3.h
>  create mode 100644 lib/arm/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/arch_gicv3.h
>  create mode 100644 lib/arm64/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/sysreg.h
> 
> diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
> new file mode 100644
> index 0..d529a7eb62807
> --- /dev/null
> +++ b/lib/arm/asm/arch_gicv3.h
> @@ -0,0 +1,184 @@
> +/*
> + * All ripped off from arch/arm/include/asm/arch_gicv3.h

So I was wondering if - from a test suite's perspective - it's really
clever to pull in copies of Linux headers here.
First it's really a lot of text we pull in while not using most of it
(at least now). Also they keep changing (4.9-rc1 saw so me changes, for
instance). So do we update them?

But more importantly those headers are also used in the emulation code,
so we would just copy any bugs or typos and would probably not detect
them here. IIRC there was a fix for a bitmask lately.
It's probably fine to copy the register offsets, but anything that
defines Linux specific things like default priorities or more complex
macros should be avoided, I think. This just makes kvm-unit-test copying
Linux behaviour.

Maybe we stick to the Linux naming, but pull in only the fields as we
need them? This would both limit the amount of lines being merged, as
would simplify the review effort (and quality), as people would just
need to look at a very limited number of defines, allowing them to
actually check it against the specification?

I gave this a try and could reduce the header files significantly.
Please let me know if you need any bits from this effort to be shared.

> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_ARCH_GICV3_H_
> +#define _ASMARM_ARCH_GICV3_H_
> +
> +#ifndef __ASSEMBLY__
> +
> +#include 
> +#include 
> +#include 
> +
> +#define __stringify xstr
> +
> +
> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2)p15, Op1, %0, CRn, CRm, Op2
> +#define __ACCESS_CP15_64(Op1, CRm)   p15, Op1, %Q0, %R0, CRm
> +
> +#define ICC_EOIR1__ACCESS_CP15(c12, 0, c12, 1)
> +#define ICC_DIR  __ACCESS_CP15(c12, 0, c11, 1)
> +#define ICC_IAR1 __ACCESS_CP15(c12, 0, c12, 0)
> +#define ICC_SGI1R__ACCESS_CP15_64(0, c12)
> +#define ICC_PMR  __ACCESS_CP15(c4, 0, c6, 0)
> +#define ICC_CTLR __ACCESS_CP15(c12, 0, c12, 4)
> +#define ICC_SRE  __ACCESS_CP15(c12, 0, c12, 5)
> +#define ICC_IGRPEN1  __ACCESS_CP15(c12, 0, c12, 7)
> +
> +#define ICC_HSRE __ACCESS_CP15(c12, 4, c9, 5)
> +
> +#define ICH_VSEIR__ACCESS_CP15(c12, 4, c9, 4)
> +#define ICH_HCR  __ACCESS_CP15(c12, 4, c11, 0)
> +#define ICH_VTR  __ACCESS_CP15(c12, 4, c11, 1)
> +#define ICH_MISR __ACCESS_CP15(c12, 4, c11, 2)
> +#define ICH_EISR __ACCESS_CP15(c12, 4, c11, 3)
> +#define ICH_ELSR __ACCESS_CP15(c12, 4, c11, 5)
> +#define ICH_VMCR __ACCESS_CP15(c12, 4, c11, 7)
> +
> +#define __LR0(x) __ACCESS_CP15(c12, 4, c12, x)
> +#define __LR8(x) __ACCESS_CP15(c12, 4, c13, x)

So for instance we clearly don't need those defines (the list registers
being hypervisor only).

> +
> +#define ICH_LR0  __LR0(0)
> +#define ICH_LR1  __LR0(1)
> +#define ICH_LR2  __LR0(2)
> +#define ICH_LR3  __LR0(3)
> +#define ICH_LR4  __LR0(4)
> +#define ICH_LR5  __LR0(5)
> +#define ICH_LR6  __LR0(6)
> +#define ICH_LR7  __LR0(7)
> +#define ICH_LR8  __LR8(0)
> +#define ICH_LR9  __LR8(1)
> +#define ICH_LR10 __LR8(2)
> +#define ICH_LR11 __LR8(3)
> +#define ICH_LR12 __LR8(4)
> +#define ICH_LR13 __LR8(5)
> +#define ICH_LR14 __LR8(6)
> +#define ICH_LR15 __LR8(7)

Re: [Qemu-devel] [kvm-unit-tests PATCH v5 09/11] arm/arm64: add initial gicv3 support

[Andre Przywara]

Hi,

On 10/11/16 17:21, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> 
> ---
> v5: use modern register names [Andre]
> v4:
>  - only take defines from kernel we need now [Andre]
>  - simplify enable by not caring if we reinit the distributor [drew]
> v2:
>  - configure irqs as NS GRP1
> ---
>  lib/arm/asm/arch_gicv3.h   | 42 +
>  lib/arm/asm/gic-v3.h   | 94 
> ++
>  lib/arm/asm/gic.h  |  6 ++-
>  lib/arm/gic.c  | 65 
>  lib/arm64/asm/arch_gicv3.h | 44 ++
>  lib/arm64/asm/gic-v3.h |  1 +
>  lib/arm64/asm/sysreg.h | 44 ++
>  7 files changed, 294 insertions(+), 2 deletions(-)
>  create mode 100644 lib/arm/asm/arch_gicv3.h
>  create mode 100644 lib/arm/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/arch_gicv3.h
>  create mode 100644 lib/arm64/asm/gic-v3.h
>  create mode 100644 lib/arm64/asm/sysreg.h
> 
> diff --git a/lib/arm/asm/arch_gicv3.h b/lib/arm/asm/arch_gicv3.h
> new file mode 100644
> index ..81a1e5f6c29c
> --- /dev/null
> +++ b/lib/arm/asm/arch_gicv3.h
> @@ -0,0 +1,42 @@
> +/*
> + * All ripped off from arch/arm/include/asm/arch_gicv3.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_ARCH_GICV3_H_
> +#define _ASMARM_ARCH_GICV3_H_
> +
> +#ifndef __ASSEMBLY__
> +#include 
> +#include 
> +#include 
> +
> +#define __stringify xstr
> +
> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2)p15, Op1, %0, CRn, CRm, Op2
> +
> +#define ICC_PMR  __ACCESS_CP15(c4, 0, c6, 0)
> +#define ICC_IGRPEN1  __ACCESS_CP15(c12, 0, c12, 7)
> +
> +static inline void gicv3_write_pmr(u32 val)
> +{
> + asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val));
> +}
> +
> +static inline void gicv3_write_grpen1(u32 val)
> +{
> + asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val));
> + isb();
> +}
> +
> +static inline u64 gicv3_read_typer(const volatile void __iomem *addr)

It may be worth to add that this is for GICR_TYPER (or GITS_TYPER),
because GICD_TYPER is 32-bit only.
Or to make the naming generic (because the code actually is), along the
lines of read_64bit_reg or the like?

> +{
> + u64 val = readl(addr);
> + val |= (u64)readl(addr + 4) << 32;
> + return val;
> +}
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* _ASMARM_ARCH_GICV3_H_ */
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> new file mode 100644
> index ..e0f303d82508
> --- /dev/null
> +++ b/lib/arm/asm/gic-v3.h
> @@ -0,0 +1,94 @@
> +/*
> + * All GIC* defines are lifted from include/linux/irqchip/arm-gic-v3.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#ifndef _ASMARM_GIC_V3_H_
> +#define _ASMARM_GIC_V3_H_
> +
> +#ifndef _ASMARM_GIC_H_
> +#error Do not directly include . Include 
> +#endif
> +
> +#define GICD_CTLR_RWP(1U << 31)
> +#define GICD_CTLR_ARE_NS (1U << 4)
> +#define GICD_CTLR_ENABLE_G1A (1U << 1)
> +#define GICD_CTLR_ENABLE_G1  (1U << 0)

+1 to Alex for adding a comment noting the non-secure view here.

> +
> +/* Re-Distributor registers, offsets from RD_base */
> +#define GICR_TYPER   0x0008
> +
> +#define GICR_TYPER_LAST  (1U << 4)
> +
> +/* Re-Distributor registers, offsets from SGI_base */
> +#define GICR_IGROUPR0GICD_IGROUPR
> +#define GICR_ISENABLER0  GICD_ISENABLER
> +#define GICR_IPRIORITYR0 GICD_IPRIORITYR
> +
> +#include 
> +
> +#ifndef __ASSEMBLY__
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct gicv3_data {
> + void *dist_base;
> + void *redist_base[NR_CPUS];
> + unsigned int irq_nr;
> +};
> +extern struct gicv3_data gicv3_data;
> +
> +#define gicv3_dist_base()(gicv3_data.dist_base)
> +#define gicv3_redist_base()  
> (gicv3_data.redist_base[smp_processor_id()])
> +#define gicv3_sgi_base() 
> (gicv3_data.redist_base[smp_processor_id()] + SZ_64K)
> +
> +extern int gicv3_init(void);
> +extern void gicv3_enable_defaults(void);
> +extern void gicv3_set_redist_base(void);
> +
> +static inline void gicv3_do_wait_for_rwp(void *base)
> +{
> + int count = 10; /* 1s */
> +
> + while (readl(base + GICD_CTLR) & GICD_CTLR_RWP) {
> + if (!--count) {
> + printf("GICv3: RWP timeout!\n");
> + abort();
> + }
> + cpu_relax();
> + udelay(10);
> + };
> +}
> +
> +static inline void gicv3_dist_wait_for_rwp(void)
> +{
> + gicv3_do_wait_for_rwp(gicv3_dist_base());
> +}
> +
> +static inline void