Re: [Qemu-devel] [BUG] QEMU crashes when 64bit BAR is present

2012-12-04 Thread Gerd Hoffmann
  Hi,

> And qemu error output is:
> qemu: /home/akorolev/qemu-kvm/exec.c:2255: register_subpage: Assertion 
> `existing->mr->subpage || existing->mr == &io_mem_unassigned' failed.
> 
> Guest OS is Centos 5.5 and log is pretty boring, as qemu crashes before Linux 
> can report an issue.

Where does it crash? seabios? linux kernel?
Still reproducable with 1.3.0?

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

2012-12-04 Thread Peter Crosthwaite
Hi Gerd, Liming,

On Tue, Dec 4, 2012 at 6:15 PM, Gerd Hoffmann  wrote:
>   Hi,
>
>> Gerd,
>>
>> Is there any documentation out there on how to tell QEMU on command
>> line which EHCI you want your usb-storage to attach to?
>
> docs/usb2.txt has some examples, although those are pc-centric where the
> ehci is created on the command line.  The problem with zynx is that both
> ehci controllers get the same (default) name so picking one by name
> doesn't work.
>
> Guess we haver to figure a way to explicitly name those devices (so the
> busses are named too), especially in case a board has two identical
> ones.  Maybe sysbus_create_simple needs an additional argument or a
> sysbus_create_simple_with_id variant.
>

Ive taken a more manual approach and just used the manual
qdev_create() flow first to get up and running:

--- a/hw/xilinx_zynq.c
+++ b/hw/xilinx_zynq.c
@@ -162,12 +162,28 @@ static void zynq_init(QEMUMachineInitArgs *args)
 pic[n] = qdev_get_gpio_in(dev, n);
 }

+dev = qdev_create(NULL, "xlnx.ps7-usb");
+dev->id = "zynq-usb-0";
+qdev_init_nofail(dev);
+busdev = sysbus_from_qdev(dev);
+sysbus_mmio_map(busdev, 0, 0xE0002000);
+sysbus_connect_irq(busdev, 0, pic[53-IRQ_OFFSET]);
+
+dev = qdev_create(NULL, "xlnx.ps7-usb");
+dev->id = "zynq-usb-1";
+busdev = sysbus_from_qdev(dev);
+qdev_init_nofail(dev);
+sysbus_mmio_map(busdev, 0, 0xE0003000);
+sysbus_connect_irq(busdev, 0, pic[76-IRQ_OFFSET]);
+
 zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET], false);
 zynq_init_spi_flashes(0xE0007000, pic[81-IRQ_OFFSET], false);
 zynq_init_spi_flashes(0xE000D000, pic[51-IRQ_OFFSET], true);

+#if 0
 sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
 sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
+#endif

This works and gives the two USBs different names, and I can attach to
each on command lines using bus=zynq-usb-0.0 and bus=zynq-usb-0.1.

We can probably now think about a more palatable way to do this,
perhaps your sysbus_create_simple_id() approach.

Regards,
Peter

> cheers,
>   Gerd
>
>



Re: [Qemu-devel] [PATCH] virtfs-proxy-helper: check return code of setfsgid/setfsuid

2012-12-04 Thread M. Mohan Kumar
"Aneesh Kumar K.V"  writes:

> I found this to be confusing. How about avoiding all those pointers, something
> like below ? If you are ok can I add the signed-off-by for this ? I can
> test this and get a pull request out with the build fix.
>
> commit 24cc9f0d07c2a505bfafbdcb72006f2eda1288a4
> Author: Paolo Bonzini 
> Date:   Thu Oct 11 14:20:23 2012 +0200
>
> virtfs-proxy-helper: use setresuid and setresgid
> 
> The setfsuid and setfsgid system calls are obscure and they complicate
> the error checking (that glibc's warn_unused_result "feature" forces
> us to do).  Switch to the standard setresuid and setresgid functions.
>
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index f9a8270..49ab0eb 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -272,31 +272,59 @@ static int send_status(int sockfd, struct iovec *iovec, 
> int status)
>  /*
>   * from man 7 capabilities, section
>   * Effect of User ID Changes on Capabilities:
> - * 4. If the file system user ID is changed from 0 to nonzero (see 
> setfsuid(2))
> - * then the following capabilities are cleared from the effective set:
> - * CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH,  CAP_FOWNER, CAP_FSETID,
> - * CAP_LINUX_IMMUTABLE  (since  Linux 2.2.30), CAP_MAC_OVERRIDE, and 
> CAP_MKNOD
> - * (since Linux 2.2.30). If the file system UID is changed from nonzero to 0,
> - * then any of these capabilities that are enabled in the permitted set
> - * are enabled in the effective set.
> + * If the effective user ID is changed from nonzero to 0, then the permitted
> + * set is copied to the effective set.  If the effective user ID is changed
> + * from 0 to nonzero, then all capabilities are are cleared from the 
> effective
> + * set.
> + *
> + * The setfsuid/setfsgid man pages warn that changing the effective user ID 
> may
> + * expose the program to unwanted signals, but this is not true anymore: for 
> an
> + * unprivileged (without CAP_KILL) program to send a signal, the real or
> + * effective user ID of the sending process must equal the real or saved user
> + * ID of the target process.  Even when dropping privileges, it is enough to
> + * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
> + * be exposed to signals.  So just use setresuid/setresgid.
>   */
> -static int setfsugid(int uid, int gid)
> +static int setugid(int uid, int gid, int suid, int sgid)
>  {
> +int retval;
> +
>  /*
> - * We still need DAC_OVERRIDE because  we don't change
> + * We still need DAC_OVERRIDE because we don't change
>   * supplementary group ids, and hence may be subjected DAC rules
>   */
>  cap_value_t cap_list[] = {
>  CAP_DAC_OVERRIDE,
>  };
>
> -setfsgid(gid);
> -setfsuid(uid);
> +if (setresuid(-1, uid, suid) == -1) {
> +retval = -errno;
> +goto err_out;
> +}
> +if (setresgid(-1, gid, sgid) == -1) {
> +retval = -errno;
> +goto err_suid;
> +}
>

After changing the order of setresuid and setresgid this patch works as
expected. Please move setresgid before setresuid.

Tested-by: M. Mohan Kumar 




[Qemu-devel] [PATCH v1 2/3] xilinx_uartlite: suppress "cannot receive message"

2012-12-04 Thread Peter Crosthwaite
This message is not an error condition, its just informing the user that
the device is corking the uart traffic to not drop characters.

Reported-by: Jason Wu 
Signed-off-by: Peter Crosthwaite 
---

 hw/xilinx_uartlite.c |6 +-
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/hw/xilinx_uartlite.c b/hw/xilinx_uartlite.c
index d20fc41..f890f23 100644
--- a/hw/xilinx_uartlite.c
+++ b/hw/xilinx_uartlite.c
@@ -182,12 +182,8 @@ static void uart_rx(void *opaque, const uint8_t *buf, int 
size)
 static int uart_can_rx(void *opaque)
 {
 struct xlx_uartlite *s = opaque;
-int r;
 
-r = s->rx_fifo_len < sizeof(s->rx_fifo);
-if (!r)
-printf("cannot receive!\n");
-return r;
+return s->rx_fifo_len < sizeof(s->rx_fifo);
 }
 
 static void uart_event(void *opaque, int event)
-- 
1.7.0.4




[Qemu-devel] [PATCH v1 3/3] xilinx_uartlite: Accept input after rx FIFO pop

2012-12-04 Thread Peter Crosthwaite
The device return false from the can receive function when the FIFO is
full. This mean the device should check for buffered input whenever a byte is
popped from the FIFO.

Reported-by: Jason Wu 
Signed-off-by: Peter Crosthwaite 
---

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

diff --git a/hw/xilinx_uartlite.c b/hw/xilinx_uartlite.c
index f890f23..02c5850 100644
--- a/hw/xilinx_uartlite.c
+++ b/hw/xilinx_uartlite.c
@@ -97,6 +97,7 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
 s->rx_fifo_len--;
 uart_update_status(s);
 uart_update_irq(s);
+qemu_chr_accept_input(s->chr);
 break;
 
 default:
-- 
1.7.0.4




[Qemu-devel] [PATCH v1 1/3] xilinx_axienet: Implement R_IS behaviour

2012-12-04 Thread Peter Crosthwaite
The interrupt status register R_IS is the standard clear-on-write behaviour.
This was unimplemented and defaulting to updating the register to the written
value. Implemented clear-on-write.

Reported-by: Jason Wu 
Signed-off-by: Peter Crosthwaite 
---

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

diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index baae02b..f2e3bf1 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -591,6 +591,10 @@ static void enet_write(void *opaque, hwaddr addr,
 s->maddr[s->fmi & 3][addr & 1] = value;
 break;
 
+case R_IS:
+s->regs[addr] &= ~value;
+break;
+
 case 0x8000 ... 0x83ff:
 s->ext_mtable[addr - 0x8000] = value;
 break;
-- 
1.7.0.4




[Qemu-devel] [PATCH v1 0/3] Xilinx Microblaze IP updates

2012-12-04 Thread Peter Crosthwaite
Minor fixes to xilinx microblaze IP.

Peter Crosthwaite (3):
  xilinx_axienet: Implement R_IS behaviour
  xilinx_uartlite: suppress "cannot receive message"
  xilinx_uartlite: Accept input after rx FIFO pop

 hw/xilinx_axienet.c  |4 
 hw/xilinx_uartlite.c |7 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)




Re: [Qemu-devel] [PULL for-1.3 0/3] seabios: q35 update

2012-12-04 Thread Gerd Hoffmann
On 12/04/12 18:14, Gabriel L. Somlo wrote:
> On Tue, Dec 04, 2012 at 05:56:55PM +0100, Gerd Hoffmann wrote:
>> Ok, and how does the RTC look like on your MacPro?
> 
> Device (RTC)
> {
> Name (_HID, EisaId ("PNP0B00"))
> Name (_CRS, ResourceTemplate ()
> {
> IO (Decode16,
> 0x0070, // Range Minimum
> 0x0070, // Range Maximum
> 0x01,   // Alignment
> 0x08,   // Length
> )
> })
> }

Ok, so no IRQ declared for the RTC.  We have IRQ 8 for both rtc and
hpet, which most likely is the root cause for the issue.  You can try
simply dropping the line for testing.  I'll try to come up with
something more clever as the hpet can be disabled in which case we
should keep irq8 assigned to rtc.

cheers,
  Gerd



[Qemu-devel] [PATCH] cadance_uart: Accept input after rx FIFO pop

2012-12-04 Thread Peter Crosthwaite
The device return false from the can receive function when the FIFO is
full. This mean the device should check for buffered input whenever a byte is
popped from the FIFO.

Reported-by: Jason Wu 
Signed-off-by: Peter Crosthwaite 
---
 hw/cadence_uart.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/cadence_uart.c b/hw/cadence_uart.c
index 686e617..a6196a2 100644
--- a/hw/cadence_uart.c
+++ b/hw/cadence_uart.c
@@ -343,6 +343,7 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
 if (!s->rx_count) {
 s->r[R_SR] |= UART_SR_INTR_REMPTY;
 }
+qemu_chr_accept_input(s->chr);
 } else {
 *c = 0;
 s->r[R_SR] |= UART_SR_INTR_REMPTY;
-- 
1.7.0.4




Re: [Qemu-devel] [PATCH v2 1/2] ivshmem: remove msix_write_config

2012-12-04 Thread Cam Macdonell
On Sat, Nov 24, 2012 at 8:51 PM, Liu Ping Fan  wrote:
> From: Liu Ping Fan 
>
> This logic has been integrated into pci core, so remove it.
>
> Signed-off-by: Liu Ping Fan 
Signed-off-by: Cam Macdonell 
> ---
>  hw/ivshmem.c |1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ivshmem.c b/hw/ivshmem.c
> index f6dbb21..7c8630c 100644
> --- a/hw/ivshmem.c
> +++ b/hw/ivshmem.c
> @@ -629,7 +629,6 @@ static void ivshmem_write_config(PCIDevice *pci_dev, 
> uint32_t address,
>  uint32_t val, int len)
>  {
>  pci_default_write_config(pci_dev, address, val, len);
> -msix_write_config(pci_dev, address, val, len);
>  }
>
>  static int pci_ivshmem_init(PCIDevice *dev)
> --
> 1.7.4.4
>



Re: [Qemu-devel] [PATCH v2 2/2] ivshmem: use irqfd to interrupt among VMs

2012-12-04 Thread Cam Macdonell
On Sat, Nov 24, 2012 at 8:51 PM, Liu Ping Fan  wrote:
> From: Liu Ping Fan 
>
> Using irqfd, so we can avoid switch between kernel and user when
> VMs interrupts each other.
>

Hi Liu Ping,

With this patch applied I was still seeing transitions to user-level
on the receipt of an msi interrupt.

uncomment DEBUG_IVSHMEM in hw/ivshmem.c (and fix one compile error in
the debug statement :) )

IVSHMEM: msix initialized (1 vectors)
...
IVSHMEM: interrupt on vector 0x7f2971b1d8d0 0
IVSHMEM: interrupt on vector 0x7f2971b1d8d0 0

if we're using irqfd, this should be avoided.  Here's my command-line:

-device ivshmem,chardev=nahanni,msi=on,ioeventfd=on,size=2048,use64=1,role=peer

There are two issues as I see it:
1) irqfd is not being enabled in my tests
2) the defaults handlers are still being added

One difference is that I'm using the UIO driver, which enables PCI
using pci_enable_msix as follows

pci_enable_msix(ivs_info->dev, ivs_info->msix_entries,
ivs_info->nvectors);

and succeeds

[2.651253] uio_ivshmem :00:04.0: irq 43 for MSI/MSI-X
[2.651326] MSI-X enabled

(continued below)

> Signed-off-by: Liu Ping Fan 
> ---
>  hw/ivshmem.c |   54 +-
>  1 files changed, 53 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ivshmem.c b/hw/ivshmem.c
> index 7c8630c..5709e89 100644
> --- a/hw/ivshmem.c
> +++ b/hw/ivshmem.c
> @@ -19,6 +19,7 @@
>  #include "hw.h"
>  #include "pc.h"
>  #include "pci.h"
> +#include "msi.h"
>  #include "msix.h"
>  #include "kvm.h"
>  #include "migration.h"
> @@ -83,6 +84,7 @@ typedef struct IVShmemState {
>  uint32_t vectors;
>  uint32_t features;
>  EventfdEntry *eventfd_table;
> +int *vector_virqs;
>
>  Error *migration_blocker;
>
> @@ -625,16 +627,62 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int 
> version_id)
>  return 0;
>  }
>
> +static int ivshmem_vector_use(PCIDevice *dev, unsigned vector,
> + MSIMessage msg)
> +{
> +IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
> +int virq;
> +EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
> +
> +virq = kvm_irqchip_add_msi_route(kvm_state, msg);
> +if (virq >= 0 && kvm_irqchip_add_irqfd_notifier(kvm_state, n, virq) >= 
> 0) {
> +s->vector_virqs[vector] = virq;
> +qemu_chr_add_handlers(s->eventfd_chr[vector], NULL, NULL, NULL, 
> NULL);
> +} else if (virq >= 0) {
> +kvm_irqchip_release_virq(kvm_state, virq);
> +error_report("ivshmem, can not setup irqfd\n");
> +return -1;
> +} else {
> +error_report("ivshmem, no enough msi route to setup irqfd\n");
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +static void ivshmem_vector_release(PCIDevice *dev, unsigned vector)
> +{
> +IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
> +EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
> +int virq = s->vector_virqs[vector];
> +
> +if (s->vector_virqs[vector] >= 0) {
> +kvm_irqchip_remove_irqfd_notifier(kvm_state, n, virq);
> +kvm_irqchip_release_virq(kvm_state, virq);
> +s->vector_virqs[vector] = -1;
> +}
> +}
> +
>  static void ivshmem_write_config(PCIDevice *pci_dev, uint32_t address,
>  uint32_t val, int len)
>  {
> +bool is_enabled, was_enabled = msi_enabled(pci_dev);
> +
>  pci_default_write_config(pci_dev, address, val, len);
> +is_enabled = msi_enabled(pci_dev);

Problem 1)  in my tests is_enabled is always 0, so I don't think the
irqfds are getting setup

> +if (!was_enabled && is_enabled) {
> +msix_set_vector_notifiers(pci_dev, ivshmem_vector_use,
> +ivshmem_vector_release);
> +} else if (was_enabled && !is_enabled) {
> +msix_unset_vector_notifiers(pci_dev);
> +}
>  }
>
>  static int pci_ivshmem_init(PCIDevice *dev)
>  {
>  IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
>  uint8_t *pci_conf;
> +int i;
>
>  if (s->sizearg == NULL)
>  s->ivshmem_size = 4 << 20; /* 4 MB default */
> @@ -758,7 +806,10 @@ static int pci_ivshmem_init(PCIDevice *dev)
>  }
>
>  s->dev.config_write = ivshmem_write_config;
> -
> +s->vector_virqs = g_new0(int, s->vectors);
> +for (i = 0; i < s->vectors; i++) {
> +s->vector_virqs[i] = -1;
> +}
>  return 0;
>  }
>
> @@ -770,6 +821,7 @@ static void pci_ivshmem_uninit(PCIDevice *dev)
>  migrate_del_blocker(s->migration_blocker);
>  error_free(s->migration_blocker);
>  }
> +g_free(s->vector_virqs);
>
>  memory_region_destroy(&s->ivshmem_mmio);
>  memory_region_del_subregion(&s->bar, &s->ivshmem);
> --
> 1.7.4.4
>

Problem 2)
We'll also have to not add the handlers as below if irqfd is present
otherwise we'll get double interrupts, so we'll have to add a check
here too.

/* if MSI is supported we need multiple interrupts */
if (!ivshmem_has_fea

Re: [Qemu-devel] [PATCH 5/7] target-mips: use DSP unions for unary DSP operators

2012-12-04 Thread Johnson, Eric
> -Original Message-
> From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> Sent: Friday, November 16, 2012 3:04 AM
> To: qemu-devel@nongnu.org
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH 5/7] target-mips: use DSP unions for unary
> DSP operators
> 
> This allow to reduce the number of macros.
> 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-mips/dsp_helper.c |  124 -
> -
>  1 file changed, 42 insertions(+), 82 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 931ca70..3bd2d35 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -1139,6 +1139,48 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a,
> uint32_t b)
>  #endif
> 
>  /** DSP Arithmetic Sub-class insns **/
> +#define MIPSDSP32_UNOP_ENV(name, func, element)
> \
> +target_ulong helper_##name(target_ulong rt, CPUMIPSState *env)
> \
> +{
> \
> +DSP32Value dt;
> \
> +unsigned int i, n;
> \
> +
> \
> +n = sizeof(DSP32Value) / sizeof(dt.element[0]);
> \
> +dt.sw[0] = rt;
> \
> +
> \
> +for (i = 0 ; i < n ; i++) {
> \
> +dt.element[i] = mipsdsp_##func(dt.element[i], env);
> \
> +}
> \
> +
> \
> +return (int32_t)dt.sw[0];
> \
> +}
> +MIPSDSP32_UNOP_ENV(absq_s_ph, sat_abs16, sh)
> +MIPSDSP32_UNOP_ENV(absq_s_qb, sat_abs8, sb)
> +MIPSDSP32_UNOP_ENV(absq_s_w, sat_abs32, sw)
> +#undef MIPSDSP32_UNOP_ENV
> +
> +#if defined(TARGET_MIPS64)
> +#define MIPSDSP64_UNOP_ENV(name, func, element)
> \
> +target_ulong helper_##name(target_ulong rt, CPUMIPSState *env)
> \
> +{
> \
> +DSP64Value dt;
> \
> +unsigned int i, n;
> \
> +
> \
> +n = sizeof(DSP64Value) / sizeof(dt.element[0]);
> \
> +dt.sl[0] = rt;
> \
> +
> \
> +for (i = 0 ; i < n ; i++) {
> \
> +dt.element[i] = mipsdsp_##func(dt.element[i], env);
> \
> +}
> \
> +
> \
> +return dt.sl[0];
> \
> +}
> +MIPSDSP64_UNOP_ENV(absq_s_ob, sat_abs8, sb)
> +MIPSDSP64_UNOP_ENV(absq_s_qh, sat_abs16, sh)
> +MIPSDSP64_UNOP_ENV(absq_s_pw, sat_abs32, sw)
> +#undef MIPSDSP64_UNOP_ENV
> +#endif
> +
>  #define MIPSDSP32_BINOP(name, func, element)
> \
>  target_ulong helper_##name(target_ulong rs, target_ulong rt)
> \
>  {
> \
> @@ -1260,16 +1302,6 @@ MIPSDSP64_BINOP_ENV(subu_s_qh, satu16_sub_u16_u16,
> uh);
> 
>  #endif
> 
> -target_ulong helper_absq_s_w(target_ulong rt, CPUMIPSState *env)
> -{
> -uint32_t rd;
> -
> -rd = mipsdsp_sat_abs32(rt, env);
> -
> -return (target_ulong)rd;
> -}
> -
> -
>  #define SUBUH_QB(name, var) \
>  target_ulong helper_##name##_qb(target_ulong rs, target_ulong rt) \
>  { \
> @@ -1377,78 +1409,6 @@ target_ulong helper_raddu_l_ob(target_ulong rs)
>  }
>  #endif
> 
> -target_ulong helper_absq_s_qb(target_ulong rt, CPUMIPSState *env)
> -{
> -uint8_t tempD, tempC, tempB, tempA;
> -
> -MIPSDSP_SPLIT32_8(rt, tempD, tempC, tempB, tempA);
> -
> -tempD = mipsdsp_sat_abs8(tempD, env);
> -tempC = mipsdsp_sat_abs8(tempC, env);
> -tempB = mipsdsp_sat_abs8(tempB, env);
> -tempA = mipsdsp_sat_abs8(tempA, env);
> -
> -return MIPSDSP_RETURN32_8(tempD, tempC, tempB, tempA);
> -}
> -
> -target_ulong helper_absq_s_ph(target_ulong rt, CPUMIPSState *env)
> -{
> -uint16_t tempB, tempA;
> -
> -MIPSDSP_SPLIT32_16(rt, tempB, tempA);
> -
> -tempB = mipsdsp_sat_abs16 (tempB, env);
> -tempA = mipsdsp_sat_abs16 (tempA, env);
> -
> -return MIPSDSP_RETURN32_16(tempB, tempA);
> -}
> -
> -#if defined(TARGET_MIPS64)
> -target_ulong helper_absq_s_ob(target_ulong rt, CPUMIPSState *env)
> -{
> -int i;
> -int8_t temp[8];
> -uint64_t result;
> -
> -for (i = 0; i < 8; i++) {
> -temp[i] = (rt >> (8 * i)) & MIPSDSP_Q0;
> -temp[i] = mipsdsp_sat_abs8(temp[i], env);
> -}
> -
> -for (i = 0; i < 8; i++) {
> -result = (uint64_t)(uint8_t)temp[i] << (8 * i);
> -}
> -
> -return result;
> -}
> -
> -target_ulong helper_absq_s_qh(target_ulong rt, CPUMIPSState *env)
> -{
> -int16_t tempD, tempC, tempB, tempA;
> -
> -MIPSDSP_SPLIT64_16(rt, tempD, tempC, tempB, tempA);
> -
> -tempD = mipsdsp_sat_abs16(tempD, env);
> -tempC = mipsdsp_sat_abs16(tempC, env);
> -tempB = mipsdsp_sat_abs16(tempB, env);
> -tempA = mipsdsp_sat_abs16(tempA, env);
> -
> -return MIPSDSP_RETURN64_16(tempD, tempC, tempB, tempA);
> -}
> -
> -target_ulong helper_absq_s_pw(target_ulong rt, CPUMIPSState *env)
> -{
> -int32_t tempB, tempA;
> -
> -MIPSDSP_SPLIT64_32(rt, tempB, tempA);
> -
> -tempB = mipsdsp_sat_abs32(tempB, env);
> -tempA = mipsdsp_sat_abs32(tempA, env);
> -
> -return MIPSDSP_RETURN64_32(tempB, tempA);
> -}
> -#endif
> -
>  #define PRECR_QB_PH(name, a, b)\
>  target_ulong helper_##name##_qb_ph(target_ulong rs, target_ulong rt) \
>  { 

Re: [Qemu-devel] [PATCH 7/7] target-mips: implement DSP (d)append sub-class with TCG

2012-12-04 Thread Johnson, Eric
> -Original Message-
> From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> Sent: Friday, November 16, 2012 3:04 AM
> To: qemu-devel@nongnu.org
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH 7/7] target-mips: implement DSP (d)append
> sub-class with TCG
> 
> DSP instruction from the (d)append sub-class can be implemented with
> TCG. Use a different function for these instructions are they are quite
> different from compare-pick sub-class.
> 
> Fix BALIGN instruction for negative value, where the value should be
> zero-extended before being shift to the right.
> 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-mips/dsp_helper.c |   67 ---
>  target-mips/helper.h |   13 -
>  target-mips/translate.c  |  133 ++---
> -
>  3 files changed, 87 insertions(+), 126 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 474c249..22bbfa1 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3140,73 +3140,6 @@ PICK_INSN(pick_pw, 2, MIPSDSP_LLO, 32, 0);
>  #endif
>  #undef PICK_INSN
> 
> -#define APPEND_INSN(name, ret_32) \
> -target_ulong helper_##name(target_ulong rt, target_ulong rs, uint32_t sa)
> \
> -{
> \
> -target_ulong temp;
> \
> -
> \
> -if (ret_32) {
> \
> -temp = ((rt & MIPSDSP_LLO) << sa) |
> \
> -   ((rs & MIPSDSP_LLO) & ((0x01 << sa) - 1));
> \
> -temp = (target_long)(int32_t)(temp & MIPSDSP_LLO);
> \
> -} else {
> \
> -temp = (rt << sa) | (rs & ((0x01 << sa) - 1));
> \
> -}
> \
> -
> \
> -return temp;
> \
> -}
> -
> -APPEND_INSN(append, 1);
> -#ifdef TARGET_MIPS64
> -APPEND_INSN(dappend, 0);
> -#endif
> -#undef APPEND_INSN
> -
> -#define PREPEND_INSN(name, or_val, ret_32)\
> -target_ulong helper_##name(target_ulong rs, target_ulong rt,  \
> -   uint32_t sa)   \
> -{ \
> -sa |= or_val; \
> -  \
> -if (1) {  \
> -return (target_long)(int32_t)(uint32_t)   \
> -(((rs & MIPSDSP_LLO) << (32 - sa)) |  \
> - ((rt & MIPSDSP_LLO) >> sa)); \
> -} else {  \
> -return (rs << (64 - sa)) | (rt >> sa);\
> -} \
> -}
> -
> -PREPEND_INSN(prepend, 0, 1);
> -#ifdef TARGET_MIPS64
> -PREPEND_INSN(prependw, 0, 0);
> -PREPEND_INSN(prependd, 0x20, 0);
> -#endif
> -#undef PREPEND_INSN
> -
> -#define BALIGN_INSN(name, filter, ret32) \
> -target_ulong helper_##name(target_ulong rs, target_ulong rt, uint32_t bp)
> \
> -{
> \
> -bp = bp & 0x03;
> \
> -
> \
> -if ((bp & 1) == 0) {
> \
> -return rt;
> \
> -} else {
> \
> -if (ret32) {
> \
> -return (target_long)(int32_t)((rt << (8 * bp)) |
> \
> -  (rs >> (8 * (4 - bp;
> \
> -} else {
> \
> -return (rt << (8 * bp)) | (rs >> (8 * (8 - bp)));
> \
> -}
> \
> -}
> \
> -}
> -
> -BALIGN_INSN(balign, 0x03, 1);
> -#if defined(TARGET_MIPS64)
> -BALIGN_INSN(dbalign, 0x07, 0);
> -#endif
> -#undef BALIGN_INSN
> -
>  target_ulong helper_packrl_ph(target_ulong rs, target_ulong rt)
>  {
>  uint32_t rsl, rth;
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index acf9ebd..4373ac5 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -654,19 +654,6 @@ DEF_HELPER_FLAGS_3(pick_ob, 0, tl, tl, tl, env)
>  DEF_HELPER_FLAGS_3(pick_qh, 0, tl, tl, tl, env)
>  DEF_HELPER_FLAGS_3(pick_pw, 0, tl, tl, tl, env)
>  #endif
> -DEF_HELPER_FLAGS_3(append, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(dappend, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
> -DEF_HELPER_FLAGS_3(prepend, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(prependd, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -DEF_HELPER_FLAGS_3(prependw, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
> -DEF_HELPER_FLAGS_3(balign, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(dbalign, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
>  DEF_HELPER_FLAGS_2(packrl_ph, TCG_CALL_NO_RWG_SE, tl, tl, tl)
>  #if defined(TARGET_MIPS64)
>  DEF_HELPER_FLAGS_2(packrl_pw, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 910dd16..624d5a5 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -336,7 +336,7 @@ enum {
>  /* DSP Bit/Manipulation Sub-class */
>  OPC_IN

Re: [Qemu-devel] [PATCH 4/7] target-mips: use DSP unions for binary DSP operators

2012-12-04 Thread Johnson, Eric
> -Original Message-
> From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> Sent: Friday, November 16, 2012 3:04 AM
> To: qemu-devel@nongnu.org
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH 4/7] target-mips: use DSP unions for binary
> DSP operators
> 
> This allow to reduce the number of macros.
> 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-mips/dsp_helper.c |  384 ++---
> -
>  1 file changed, 116 insertions(+), 268 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 8015d8d..931ca70 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -1107,7 +1107,6 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t a,
> uint32_t b)
>  b = num & MIPSDSP_LO;   \
>  } while (0)
> 
> -#define MIPSDSP_RETURN32(a) ((target_long)(int32_t)a)
>  #define MIPSDSP_RETURN32_8(a, b, c, d)  ((target_long)(int32_t) \
>   (((uint32_t)a << 24) | \
>   (((uint32_t)b << 16) | \
> @@ -1140,119 +1139,127 @@ static inline int32_t mipsdsp_cmpu_lt(uint32_t
> a, uint32_t b)
>  #endif
> 
>  /** DSP Arithmetic Sub-class insns **/
> -#define ARITH_PH(name, func)  \
> -target_ulong helper_##name##_ph(target_ulong rs, target_ulong rt) \
> -{ \
> -uint16_t  rsh, rsl, rth, rtl, temph, templ;   \
> -  \
> -MIPSDSP_SPLIT32_16(rs, rsh, rsl); \
> -MIPSDSP_SPLIT32_16(rt, rth, rtl); \
> -  \
> -temph = mipsdsp_##func(rsh, rth); \
> -templ = mipsdsp_##func(rsl, rtl); \
> -  \
> -return MIPSDSP_RETURN32_16(temph, templ); \
> -}
> -
> -#define ARITH_PH_ENV(name, func)  \
> -target_ulong helper_##name##_ph(target_ulong rs, target_ulong rt, \
> -CPUMIPSState *env)\
> -{ \
> -uint16_t  rsh, rsl, rth, rtl, temph, templ;   \
> -  \
> -MIPSDSP_SPLIT32_16(rs, rsh, rsl); \
> -MIPSDSP_SPLIT32_16(rt, rth, rtl); \
> -  \
> -temph = mipsdsp_##func(rsh, rth, env);\
> -templ = mipsdsp_##func(rsl, rtl, env);\
> -  \
> -return MIPSDSP_RETURN32_16(temph, templ); \
> -}
> -
> -
> -ARITH_PH_ENV(addq, add_i16);
> -ARITH_PH_ENV(addq_s, sat_add_i16);
> -ARITH_PH_ENV(addu, add_u16);
> -ARITH_PH_ENV(addu_s, sat_add_u16);
> -
> -ARITH_PH(addqh, rshift1_add_q16);
> -ARITH_PH(addqh_r, rrshift1_add_q16);
> -
> -ARITH_PH_ENV(subq, sub_i16);
> -ARITH_PH_ENV(subq_s, sat16_sub);
> -ARITH_PH_ENV(subu, sub_u16_u16);
> -ARITH_PH_ENV(subu_s, satu16_sub_u16_u16);
> -
> -ARITH_PH(subqh, rshift1_sub_q16);
> -ARITH_PH(subqh_r, rrshift1_sub_q16);
> -
> -#undef ARITH_PH
> -#undef ARITH_PH_ENV
> +#define MIPSDSP32_BINOP(name, func, element)
> \
> +target_ulong helper_##name(target_ulong rs, target_ulong rt)
> \
> +{
> \
> +DSP32Value ds, dt;
> \
> +unsigned int i, n;
> \
> +
> \
> +n = sizeof(DSP32Value) / sizeof(ds.element[0]);
> \
> +ds.sw[0] = rs;
> \
> +dt.sw[0] = rt;
> \
> +
> \
> +for (i = 0 ; i < n ; i++) {
> \
> +ds.element[i] = mipsdsp_##func(ds.element[i], dt.element[i]);
> \
> +}
> \
> +
> \
> +return (int32_t)ds.sw[0];
> \
> +}
> +MIPSDSP32_BINOP(addqh_ph, rshift1_add_q16, sh);
> +MIPSDSP32_BINOP(addqh_r_ph, rrshift1_add_q16, sh);
> +MIPSDSP32_BINOP(addqh_r_w, rrshift1_add_q32, sw);
> +MIPSDSP32_BINOP(addqh_w, rshift1_add_q32, sw);
> +MIPSDSP32_BINOP(adduh_qb, rshift1_add_u8, ub);
> +MIPSDSP32_BINOP(adduh_r_qb, rrshift1_add_u8, ub);
> +MIPSDSP32_BINOP(subqh_ph, rshift1_sub_q16, sh);
> +MIPSDSP32_BINOP(subqh_r_ph, rrshift1_sub_q16, sh);
> +MIPSDSP32_BINOP(subqh_r_w, rrshift1_sub_q32, sw);
> +MIPSDSP32_BINOP(subqh_w, rshift1_sub_q32, sw);
> +#undef MIPSDSP32_BINOP
> +
> +#define MIPSDSP32_BINOP_ENV(name, func, element)
> \
> +target_ulong helper_##name(target_ulong rs, target_ulong rt,
> \
> +   CPUMIPSState *env)
> \
> +{
> \
> +DSP32Value ds, dt;
> \
> +unsigned int i, n;
> \
> +
> \
> +n = sizeof(DSP32Value) / sizeof(ds.element[0]);
> \
> +ds.sw[0]

Re: [Qemu-devel] [PATCH 6/7] target-mips: use DSP unions for reduction add instructions

2012-12-04 Thread Johnson, Eric
> -Original Message-
> From: qemu-devel-bounces+ericj=mips@nongnu.org [mailto:qemu-devel-
> bounces+ericj=mips@nongnu.org] On Behalf Of Aurelien Jarno
> Sent: Friday, November 16, 2012 3:04 AM
> To: qemu-devel@nongnu.org
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH 6/7] target-mips: use DSP unions for
> reduction add instructions
> 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-mips/dsp_helper.c |   32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 3bd2d35..474c249 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -1381,31 +1381,29 @@ target_ulong helper_modsub(target_ulong rs,
> target_ulong rt)
> 
>  target_ulong helper_raddu_w_qb(target_ulong rs)
>  {
> -uint8_t  rs3, rs2, rs1, rs0;
> -uint16_t temp;
> -
> -MIPSDSP_SPLIT32_8(rs, rs3, rs2, rs1, rs0);
> -
> -temp = (uint16_t)rs3 + (uint16_t)rs2 + (uint16_t)rs1 + (uint16_t)rs0;
> +target_ulong ret = 0;
> +DSP32Value ds;
> +unsigned int i;
> 
> -return (target_ulong)temp;
> +ds.uw[0] = rs;
> +for (i = 0 ; i < 4 ; i++) {
> +ret += ds.ub[i];
> +}
> +return ret;
>  }
> 
>  #if defined(TARGET_MIPS64)
>  target_ulong helper_raddu_l_ob(target_ulong rs)
>  {
> -int i;
> -uint16_t rs_t[8];
> -uint64_t temp;
> -
> -temp = 0;
> +target_ulong ret = 0;
> +DSP64Value ds;
> +unsigned int i;
> 
> -for (i = 0; i < 8; i++) {
> -rs_t[i] = (rs >> (8 * i)) & MIPSDSP_Q0;
> -temp += (uint64_t)rs_t[i];
> +ds.ul[0] = rs;
> +for (i = 0 ; i < 8 ; i++) {
> +ret += ds.ub[i];
>  }
> -
> -return temp;
> +return ret;
>  }
>  #endif
> 
> --
> 1.7.10.4
> 

Reviewed-by: Eric Johnson 



Re: [Qemu-devel] [PATCH] xilinx_zynq: Fix wrong IRQ number of the second EHCI controller

2012-12-04 Thread Peter Crosthwaite
On Tue, Dec 4, 2012 at 4:59 PM, Liming Wang  wrote:
> The IRQ number of the second EHCI controller should be 76, not 75.
>
> Signed-off-by: Liming Wang 

Tested-by: Peter Crosthwaite 

> ---
>  hw/xilinx_zynq.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c
> index 1f12a3d..808de68 100644
> --- a/hw/xilinx_zynq.c
> +++ b/hw/xilinx_zynq.c
> @@ -167,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
>  zynq_init_spi_flashes(0xE000D000, pic[51-IRQ_OFFSET], true);
>
>  sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
> -sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[75-IRQ_OFFSET]);
> +sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
>
>  sysbus_create_simple("cadence_uart", 0xE000, pic[59-IRQ_OFFSET]);
>  sysbus_create_simple("cadence_uart", 0xE0001000, pic[82-IRQ_OFFSET]);
> --
> 1.7.9.5
>
>



[Qemu-devel] [PATCH 3/8] target-xtensa: restrict available SRs by enabled options

2012-12-04 Thread Max Filippov
Beginning with the RA-2004.1 release, SR access instructions (rsr, wsr,
xsr) are associated with their corresponding SR and raise illegal opcode
exception in case the register is not configured for the core.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h  |1 +
 target-xtensa/overlay_tool.h |4 +-
 target-xtensa/translate.c|  230 +++---
 3 files changed, 130 insertions(+), 105 deletions(-)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 068ad69..a73d32d 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -416,6 +416,7 @@ void debug_exception_env(CPUXtensaState *new_env, uint32_t 
cause);
 
 
 #define XTENSA_OPTION_BIT(opt) (((uint64_t)1) << (opt))
+#define XTENSA_OPTION_ALL (~(uint64_t)0)
 
 static inline bool xtensa_option_bits_enabled(const XtensaConfig *config,
 uint64_t opt)
diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
index 45205b8..0b47029 100644
--- a/target-xtensa/overlay_tool.h
+++ b/target-xtensa/overlay_tool.h
@@ -94,7 +94,9 @@
 XCHAL_OPTION(XCHAL_HAVE_CACHEATTR, XTENSA_OPTION_CACHEATTR) | \
 /* Other, TODO */ \
 XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \
-XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG))
+XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG) |\
+XCHAL_OPTION(XCHAL_HAVE_THREADPTR, XTENSA_OPTION_THREAD_POINTER) | \
+XCHAL_OPTION(XCHAL_HAVE_PRID, XTENSA_OPTION_PROCESSOR_ID))
 
 #ifndef XCHAL_WINDOW_OF4_VECOFS
 #define XCHAL_WINDOW_OF4_VECOFS 0x
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index c246fcb..5416aff 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -78,78 +78,102 @@ static TCGv_i32 cpu_UR[256];
 
 #include "gen-icount.h"
 
-static const char * const sregnames[256] = {
-[LBEG] = "LBEG",
-[LEND] = "LEND",
-[LCOUNT] = "LCOUNT",
-[SAR] = "SAR",
-[BR] = "BR",
-[LITBASE] = "LITBASE",
-[SCOMPARE1] = "SCOMPARE1",
-[ACCLO] = "ACCLO",
-[ACCHI] = "ACCHI",
-[MR] = "MR0",
-[MR + 1] = "MR1",
-[MR + 2] = "MR2",
-[MR + 3] = "MR3",
-[WINDOW_BASE] = "WINDOW_BASE",
-[WINDOW_START] = "WINDOW_START",
-[PTEVADDR] = "PTEVADDR",
-[RASID] = "RASID",
-[ITLBCFG] = "ITLBCFG",
-[DTLBCFG] = "DTLBCFG",
-[IBREAKENABLE] = "IBREAKENABLE",
-[CACHEATTR] = "CACHEATTR",
-[ATOMCTL] = "ATOMCTL",
-[IBREAKA] = "IBREAKA0",
-[IBREAKA + 1] = "IBREAKA1",
-[DBREAKA] = "DBREAKA0",
-[DBREAKA + 1] = "DBREAKA1",
-[DBREAKC] = "DBREAKC0",
-[DBREAKC + 1] = "DBREAKC1",
-[EPC1] = "EPC1",
-[EPC1 + 1] = "EPC2",
-[EPC1 + 2] = "EPC3",
-[EPC1 + 3] = "EPC4",
-[EPC1 + 4] = "EPC5",
-[EPC1 + 5] = "EPC6",
-[EPC1 + 6] = "EPC7",
-[DEPC] = "DEPC",
-[EPS2] = "EPS2",
-[EPS2 + 1] = "EPS3",
-[EPS2 + 2] = "EPS4",
-[EPS2 + 3] = "EPS5",
-[EPS2 + 4] = "EPS6",
-[EPS2 + 5] = "EPS7",
-[EXCSAVE1] = "EXCSAVE1",
-[EXCSAVE1 + 1] = "EXCSAVE2",
-[EXCSAVE1 + 2] = "EXCSAVE3",
-[EXCSAVE1 + 3] = "EXCSAVE4",
-[EXCSAVE1 + 4] = "EXCSAVE5",
-[EXCSAVE1 + 5] = "EXCSAVE6",
-[EXCSAVE1 + 6] = "EXCSAVE7",
-[CPENABLE] = "CPENABLE",
-[INTSET] = "INTSET",
-[INTCLEAR] = "INTCLEAR",
-[INTENABLE] = "INTENABLE",
-[PS] = "PS",
-[VECBASE] = "VECBASE",
-[EXCCAUSE] = "EXCCAUSE",
-[DEBUGCAUSE] = "DEBUGCAUSE",
-[CCOUNT] = "CCOUNT",
-[PRID] = "PRID",
-[ICOUNT] = "ICOUNT",
-[ICOUNTLEVEL] = "ICOUNTLEVEL",
-[EXCVADDR] = "EXCVADDR",
-[CCOMPARE] = "CCOMPARE0",
-[CCOMPARE + 1] = "CCOMPARE1",
-[CCOMPARE + 2] = "CCOMPARE2",
+typedef struct XtensaReg {
+const char *name;
+uint64_t opt_bits;
+} XtensaReg;
+
+#define XTENSA_REG(regname, opt) { \
+.name = (regname), \
+.opt_bits = XTENSA_OPTION_BIT(opt), \
+}
+
+#define XTENSA_REG_BITS(regname, opt) { \
+.name = (regname), \
+.opt_bits = (opt), \
+}
+
+static const XtensaReg sregnames[256] = {
+[LBEG] = XTENSA_REG("LBEG", XTENSA_OPTION_LOOP),
+[LEND] = XTENSA_REG("LEND", XTENSA_OPTION_LOOP),
+[LCOUNT] = XTENSA_REG("LCOUNT", XTENSA_OPTION_LOOP),
+[SAR] = XTENSA_REG_BITS("SAR", XTENSA_OPTION_ALL),
+[BR] = XTENSA_REG("BR", XTENSA_OPTION_BOOLEAN),
+[LITBASE] = XTENSA_REG("LITBASE", XTENSA_OPTION_EXTENDED_L32R),
+[SCOMPARE1] = XTENSA_REG("SCOMPARE1", XTENSA_OPTION_CONDITIONAL_STORE),
+[ACCLO] = XTENSA_REG("ACCLO", XTENSA_OPTION_MAC16),
+[ACCHI] = XTENSA_REG("ACCHI", XTENSA_OPTION_MAC16),
+[MR] = XTENSA_REG("MR0", XTENSA_OPTION_MAC16),
+[MR + 1] = XTENSA_REG("MR1", XTENSA_OPTION_MAC16),
+[MR + 2] = XTENSA_REG("MR2", XTENSA_OPTION_MAC16),
+[MR + 3] = XTENSA_REG("MR3", XTENSA_OPTION_MAC16),
+[WINDOW_BASE] = XTENSA_REG("WINDOW_BASE", XTENSA_OPTION_WINDOWED_REGISTER),
+[WINDOW_START] = XTENSA_REG("WINDOW_START",
+XTENSA_OPTION_WINDOWED_REGISTE

[Qemu-devel] [PATCH 6/8] target-xtensa: add SR accessibility unit tests

2012-12-04 Thread Max Filippov
Signed-off-by: Max Filippov 
---
 tests/tcg/xtensa/Makefile   |1 +
 tests/tcg/xtensa/macros.inc |2 +-
 tests/tcg/xtensa/test_sr.S  |   90 +++
 3 files changed, 92 insertions(+), 1 deletions(-)
 create mode 100644 tests/tcg/xtensa/test_sr.S

diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile
index 0ff0ccf..56cfe0f 100644
--- a/tests/tcg/xtensa/Makefile
+++ b/tests/tcg/xtensa/Makefile
@@ -45,6 +45,7 @@ TESTCASES += test_rst0.tst
 TESTCASES += test_sar.tst
 TESTCASES += test_sext.tst
 TESTCASES += test_shift.tst
+TESTCASES += test_sr.tst
 TESTCASES += test_timer.tst
 TESTCASES += test_windowed.tst
 
diff --git a/tests/tcg/xtensa/macros.inc b/tests/tcg/xtensa/macros.inc
index 23bf3e9..c9be1ce 100644
--- a/tests/tcg/xtensa/macros.inc
+++ b/tests/tcg/xtensa/macros.inc
@@ -1,7 +1,7 @@
 .macro test_suite name
 .data
 status: .word result
-result: .space 20
+result: .space 256
 .text
 .global main
 .align 4
diff --git a/tests/tcg/xtensa/test_sr.S b/tests/tcg/xtensa/test_sr.S
new file mode 100644
index 000..470c03d
--- /dev/null
+++ b/tests/tcg/xtensa/test_sr.S
@@ -0,0 +1,90 @@
+.include "macros.inc"
+
+test_suite sr
+
+.macro  sr_op sym, op_sym, op_byte, sr
+.if \sym
+\op_sym a4, \sr
+.else
+.byte 0x40, \sr, \op_byte
+.endif
+.endm
+
+.macro test_sr_op sym, mask, op, op_byte, sr
+movia4, 0
+.if (\mask)
+set_vector kernel, 0
+sr_op   \sym, \op, \op_byte, \sr
+.else
+set_vector kernel, 2f
+1:
+sr_op   \sym, \op, \op_byte, \sr
+test_fail
+2:
+reset_ps
+rsr a2, exccause
+assert  eqi, a2, 0
+rsr a2, epc1
+movia3, 1b
+assert  eq, a2, a3
+.endif
+.endm
+
+.macro  test_sr_mask sr, sym, mask
+test \sr
+test_sr_op \sym, \mask & 1, rsr, 0x03, \sr
+test_sr_op \sym, \mask & 2, wsr, 0x13, \sr
+test_sr_op \sym, \mask & 4, xsr, 0x61, \sr
+test_end
+.endm
+
+.macro  test_sr sr, conf
+test_sr_mask\sr, \conf, 7
+.endm
+
+test_sr acchi, 1
+test_sr acclo, 1
+test_sr_mask /*atomctl*/99, 0, 0
+test_sr_mask /*br*/4, 0, 0
+test_sr_mask /*cacheattr*/98, 0, 0
+test_sr ccompare0, 1
+test_sr ccount, 1
+test_sr cpenable, 1
+test_sr dbreaka0, 1
+test_sr dbreakc0, 1
+test_sr_mask debugcause, 1, 1
+test_sr depc, 1
+test_sr dtlbcfg, 1
+test_sr epc1, 1
+test_sr epc2, 1
+test_sr eps2, 1
+test_sr exccause, 1
+test_sr excsave1, 1
+test_sr excsave2, 1
+test_sr excvaddr, 1
+test_sr ibreaka0, 1
+test_sr ibreakenable, 1
+test_sr icount, 1
+test_sr icountlevel, 1
+test_sr_mask /*intclear*/227, 0, 2
+test_sr_mask /*interrupt*/226, 0, 3
+test_sr intenable, 1
+test_sr itlbcfg, 1
+test_sr lbeg, 1
+test_sr lcount, 1
+test_sr lend, 1
+test_sr litbase, 1
+test_sr m0, 1
+test_sr misc0, 1
+test_sr_mask /*prefctl*/40, 0, 0
+test_sr_mask /*prid*/235, 0, 1
+test_sr ps, 1
+test_sr ptevaddr, 1
+test_sr rasid, 1
+test_sr sar, 1
+test_sr scompare1, 1
+test_sr vecbase, 1
+test_sr windowbase, 1
+test_sr windowstart, 1
+
+test_suite_end
-- 
1.7.7.6




[Qemu-devel] [PATCH 1/8] target-xtensa: implement ATOMCTL SR

2012-12-04 Thread Max Filippov
ATOMCTL SR controls s32c1i opcode behavior depending on targeted memory
type. See ISA, 4.3.12.4 for details.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.c  |2 +
 target-xtensa/cpu.h  |   10 +++
 target-xtensa/helper.c   |   56 +++--
 target-xtensa/helper.h   |1 +
 target-xtensa/op_helper.c|   57 ++
 target-xtensa/overlay_tool.h |6 
 target-xtensa/translate.c|   13 +
 7 files changed, 131 insertions(+), 14 deletions(-)

diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index 9d01983..c6aa45e 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -48,6 +48,8 @@ static void xtensa_cpu_reset(CPUState *s)
 XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10;
 env->sregs[VECBASE] = env->config->vecbase;
 env->sregs[IBREAKENABLE] = 0;
+env->sregs[ATOMCTL] = xtensa_option_enabled(env->config,
+XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15;
 
 env->pending_irq_level = 0;
 reset_mmu(env);
diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 74e9888..d240ab7 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -65,6 +65,7 @@ enum {
 XTENSA_OPTION_FP_COPROCESSOR,
 XTENSA_OPTION_MP_SYNCHRO,
 XTENSA_OPTION_CONDITIONAL_STORE,
+XTENSA_OPTION_ATOMCTL,
 
 /* Interrupts and exceptions */
 XTENSA_OPTION_EXCEPTION,
@@ -128,6 +129,7 @@ enum {
 ITLBCFG = 91,
 DTLBCFG = 92,
 IBREAKENABLE = 96,
+ATOMCTL = 99,
 IBREAKA = 128,
 DBREAKA = 144,
 DBREAKC = 160,
@@ -193,6 +195,14 @@ enum {
 
 #define REGION_PAGE_MASK 0xe000
 
+#define PAGE_CACHE_MASK0x700
+#define PAGE_CACHE_SHIFT   8
+#define PAGE_CACHE_INVALID 0x000
+#define PAGE_CACHE_BYPASS  0x100
+#define PAGE_CACHE_WT  0x200
+#define PAGE_CACHE_WB  0x400
+#define PAGE_CACHE_ISOLATE 0x600
+
 enum {
 /* Static vectors */
 EXC_RESET,
diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index d94bae2..ecd0182 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -390,6 +390,7 @@ int xtensa_tlb_lookup(const CPUXtensaState *env, uint32_t 
addr, bool dtlb,
 static unsigned mmu_attr_to_access(uint32_t attr)
 {
 unsigned access = 0;
+
 if (attr < 12) {
 access |= PAGE_READ;
 if (attr & 0x1) {
@@ -398,8 +399,22 @@ static unsigned mmu_attr_to_access(uint32_t attr)
 if (attr & 0x2) {
 access |= PAGE_WRITE;
 }
+
+switch (attr & 0xc) {
+case 0:
+access |= PAGE_CACHE_BYPASS;
+break;
+
+case 4:
+access |= PAGE_CACHE_WB;
+break;
+
+case 8:
+access |= PAGE_CACHE_WT;
+break;
+}
 } else if (attr == 13) {
-access |= PAGE_READ | PAGE_WRITE;
+access |= PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE;
 }
 return access;
 }
@@ -410,14 +425,17 @@ static unsigned mmu_attr_to_access(uint32_t attr)
  */
 static unsigned region_attr_to_access(uint32_t attr)
 {
-unsigned access = 0;
-if ((attr < 6 && attr != 3) || attr == 14) {
-access |= PAGE_READ | PAGE_WRITE;
-}
-if (attr > 0 && attr < 6) {
-access |= PAGE_EXEC;
-}
-return access;
+static const unsigned access[16] = {
+ [0] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_WT,
+ [1] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WT,
+ [2] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS,
+ [3] =  PAGE_EXEC | PAGE_CACHE_WB,
+ [4] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB,
+ [5] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB,
+[14] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE,
+};
+
+return access[attr & 0xf];
 }
 
 static bool is_access_granted(unsigned access, int is_write)
@@ -566,7 +584,7 @@ int xtensa_get_physical_addr(CPUXtensaState *env, bool 
update_tlb,
 } else {
 *paddr = vaddr;
 *page_size = TARGET_PAGE_SIZE;
-*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS;
 return 0;
 }
 }
@@ -599,24 +617,34 @@ static void dump_tlb(FILE *f, fprintf_function 
cpu_fprintf,
 xtensa_tlb_get_entry(env, dtlb, wi, ei);
 
 if (entry->asid) {
+static const char * const cache_text[8] = {
+[PAGE_CACHE_BYPASS >> PAGE_CACHE_SHIFT] = "Bypass",
+[PAGE_CACHE_WT >> PAGE_CACHE_SHIFT] = "WT",
+[PAGE_CACHE_WB >> PAGE_CACHE_SHIFT] = "WB",
+[PAGE_CACHE_ISOLATE >> PAGE_CACHE_SHIFT] = "Isolate",
+};
 unsigned access = attr_to_access(entry->attr);
+unsigned cache_idx = (access & PAGE_CACHE_MASK) >>
+PAGE_CACHE_SHIFT;
 
 if (print_he

[Qemu-devel] [PATCH 5/8] target-xtensa: implement MISC SR

2012-12-04 Thread Max Filippov
The Miscellaneous Special Registers Option provides zero to four scratch
registers within the processor readable and writable by RSR, WSR, and
XSR. These registers are privileged. They may be useful for some
application-specific exception and interrupt processing tasks in the
kernel. The MISC registers are undefined after reset.
See ISA, 4.7.3 for details.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h  |1 +
 target-xtensa/overlay_tool.h |1 +
 target-xtensa/translate.c|4 
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index a73d32d..08fd5bc 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -153,6 +153,7 @@ enum {
 ICOUNTLEVEL = 237,
 EXCVADDR = 238,
 CCOMPARE = 240,
+MISC = 244,
 };
 
 #define PS_INTLEVEL 0xf
diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
index 0b47029..dd4f51a 100644
--- a/target-xtensa/overlay_tool.h
+++ b/target-xtensa/overlay_tool.h
@@ -95,6 +95,7 @@
 /* Other, TODO */ \
 XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \
 XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG) |\
+XCHAL_OPTION(XCHAL_NUM_MISC_REGS > 0, XTENSA_OPTION_MISC_SR) | \
 XCHAL_OPTION(XCHAL_HAVE_THREADPTR, XTENSA_OPTION_THREAD_POINTER) | \
 XCHAL_OPTION(XCHAL_HAVE_PRID, XTENSA_OPTION_PROCESSOR_ID))
 
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index fbeac7f..48a22de 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -179,6 +179,10 @@ static const XtensaReg sregnames[256] = {
 XTENSA_OPTION_TIMER_INTERRUPT),
 [CCOMPARE + 2] = XTENSA_REG("CCOMPARE2",
 XTENSA_OPTION_TIMER_INTERRUPT),
+[MISC] = XTENSA_REG("MISC0", XTENSA_OPTION_MISC_SR),
+[MISC + 1] = XTENSA_REG("MISC1", XTENSA_OPTION_MISC_SR),
+[MISC + 2] = XTENSA_REG("MISC2", XTENSA_OPTION_MISC_SR),
+[MISC + 3] = XTENSA_REG("MISC3", XTENSA_OPTION_MISC_SR),
 };
 
 static const XtensaReg uregnames[256] = {
-- 
1.7.7.6




[Qemu-devel] [PATCH 4/8] target-xtensa: better control rsr/wsr/xsr access to SRs

2012-12-04 Thread Max Filippov
There are read-only (DEBUGCAUSE, PRID) and write-only (INTCLEAR) SRs,
and INTERRUPT/INTSET SR allows rsr/wsr, but not xsr. Raise illeagal
opcode exception on illegal access to these SRs.

Signed-off-by: Max Filippov 
---
 target-xtensa/translate.c |   49 +++-
 1 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 5416aff..fbeac7f 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -81,16 +81,27 @@ static TCGv_i32 cpu_UR[256];
 typedef struct XtensaReg {
 const char *name;
 uint64_t opt_bits;
+enum {
+SR_R = 1,
+SR_W = 2,
+SR_X = 4,
+SR_RW = 3,
+SR_RWX = 7,
+} access;
 } XtensaReg;
 
-#define XTENSA_REG(regname, opt) { \
+#define XTENSA_REG_ACCESS(regname, opt, acc) { \
 .name = (regname), \
 .opt_bits = XTENSA_OPTION_BIT(opt), \
+.access = (acc), \
 }
 
+#define XTENSA_REG(regname, opt) XTENSA_REG_ACCESS(regname, opt, SR_RWX)
+
 #define XTENSA_REG_BITS(regname, opt) { \
 .name = (regname), \
 .opt_bits = (opt), \
+.access = SR_RWX, \
 }
 
 static const XtensaReg sregnames[256] = {
@@ -151,15 +162,15 @@ static const XtensaReg sregnames[256] = {
 [EXCSAVE1 + 6] = XTENSA_REG("EXCSAVE7",
 XTENSA_OPTION_HIGH_PRIORITY_INTERRUPT),
 [CPENABLE] = XTENSA_REG("CPENABLE", XTENSA_OPTION_COPROCESSOR),
-[INTSET] = XTENSA_REG("INTSET", XTENSA_OPTION_INTERRUPT),
-[INTCLEAR] = XTENSA_REG("INTCLEAR", XTENSA_OPTION_INTERRUPT),
+[INTSET] = XTENSA_REG_ACCESS("INTSET", XTENSA_OPTION_INTERRUPT, SR_RW),
+[INTCLEAR] = XTENSA_REG_ACCESS("INTCLEAR", XTENSA_OPTION_INTERRUPT, SR_W),
 [INTENABLE] = XTENSA_REG("INTENABLE", XTENSA_OPTION_INTERRUPT),
 [PS] = XTENSA_REG_BITS("PS", XTENSA_OPTION_ALL),
 [VECBASE] = XTENSA_REG("VECBASE", XTENSA_OPTION_RELOCATABLE_VECTOR),
 [EXCCAUSE] = XTENSA_REG("EXCCAUSE", XTENSA_OPTION_EXCEPTION),
-[DEBUGCAUSE] = XTENSA_REG("DEBUGCAUSE", XTENSA_OPTION_DEBUG),
+[DEBUGCAUSE] = XTENSA_REG_ACCESS("DEBUGCAUSE", XTENSA_OPTION_DEBUG, SR_R),
 [CCOUNT] = XTENSA_REG("CCOUNT", XTENSA_OPTION_TIMER_INTERRUPT),
-[PRID] = XTENSA_REG("PRID", XTENSA_OPTION_PROCESSOR_ID),
+[PRID] = XTENSA_REG_ACCESS("PRID", XTENSA_OPTION_PROCESSOR_ID, SR_R),
 [ICOUNT] = XTENSA_REG("ICOUNT", XTENSA_OPTION_DEBUG),
 [ICOUNTLEVEL] = XTENSA_REG("ICOUNTLEVEL", XTENSA_OPTION_DEBUG),
 [EXCVADDR] = XTENSA_REG("EXCVADDR", XTENSA_OPTION_EXCEPTION),
@@ -476,7 +487,7 @@ static void gen_brcondi(DisasContext *dc, TCGCond cond,
 tcg_temp_free(tmp);
 }
 
-static void gen_check_sr(DisasContext *dc, uint32_t sr)
+static void gen_check_sr(DisasContext *dc, uint32_t sr, unsigned access)
 {
 if (!xtensa_option_bits_enabled(dc->config, sregnames[sr].opt_bits)) {
 if (sregnames[sr].name) {
@@ -485,6 +496,16 @@ static void gen_check_sr(DisasContext *dc, uint32_t sr)
 qemu_log("SR %d is not implemented\n", sr);
 }
 gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
+} else if (!(sregnames[sr].access & access)) {
+static const char * const access_text[] = {
+[SR_R] = "rsr",
+[SR_W] = "wsr",
+[SR_X] = "xsr",
+};
+assert(access < ARRAY_SIZE(access_text) && access_text[access]);
+qemu_log("SR %s is not available for %s\n", sregnames[sr].name,
+access_text[access]);
+gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
 }
 }
 
@@ -679,14 +700,6 @@ static void gen_wsr_ps(DisasContext *dc, uint32_t sr, 
TCGv_i32 v)
 gen_jumpi_check_loop_end(dc, -1);
 }
 
-static void gen_wsr_debugcause(DisasContext *dc, uint32_t sr, TCGv_i32 v)
-{
-}
-
-static void gen_wsr_prid(DisasContext *dc, uint32_t sr, TCGv_i32 v)
-{
-}
-
 static void gen_wsr_icount(DisasContext *dc, uint32_t sr, TCGv_i32 v)
 {
 if (dc->icount) {
@@ -744,8 +757,6 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 
s)
 [INTCLEAR] = gen_wsr_intclear,
 [INTENABLE] = gen_wsr_intenable,
 [PS] = gen_wsr_ps,
-[DEBUGCAUSE] = gen_wsr_debugcause,
-[PRID] = gen_wsr_prid,
 [ICOUNT] = gen_wsr_icount,
 [ICOUNTLEVEL] = gen_wsr_icountlevel,
 [CCOMPARE] = gen_wsr_ccompare,
@@ -1467,7 +1478,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 case 6: /*XSR*/
 {
 TCGv_i32 tmp = tcg_temp_new_i32();
-gen_check_sr(dc, RSR_SR);
+gen_check_sr(dc, RSR_SR, SR_X);
 if (RSR_SR >= 64) {
 gen_check_privilege(dc);
 }
@@ -1698,7 +1709,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 case 3: /*RST3*/
 switch (OP2) {
 case 0: /*RSR*/
-gen_check_sr(dc, RSR_SR);

[Qemu-devel] [PATCH 2/8] target-xtensa: implement CACHEATTR SR

2012-12-04 Thread Max Filippov
In XEA1, the Options for Memory Protection and Translation and the
corresponding TLB management instructions are not available. Instead,
functionality similar to the Region Protection Option is available
through the cache attribute register. See ISA, A.2.14 for details.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.c  |1 +
 target-xtensa/cpu.h  |2 ++
 target-xtensa/helper.c   |   21 -
 target-xtensa/overlay_tool.h |1 +
 target-xtensa/translate.c|1 +
 5 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index c6aa45e..035b07c 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -48,6 +48,7 @@ static void xtensa_cpu_reset(CPUState *s)
 XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10;
 env->sregs[VECBASE] = env->config->vecbase;
 env->sregs[IBREAKENABLE] = 0;
+env->sregs[CACHEATTR] = 0x;
 env->sregs[ATOMCTL] = xtensa_option_enabled(env->config,
 XTENSA_OPTION_ATOMCTL) ? 0x28 : 0x15;
 
diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index d240ab7..068ad69 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -94,6 +94,7 @@ enum {
 XTENSA_OPTION_REGION_PROTECTION,
 XTENSA_OPTION_REGION_TRANSLATION,
 XTENSA_OPTION_MMU,
+XTENSA_OPTION_CACHEATTR,
 
 /* Other */
 XTENSA_OPTION_WINDOWED_REGISTER,
@@ -129,6 +130,7 @@ enum {
 ITLBCFG = 91,
 DTLBCFG = 92,
 IBREAKENABLE = 96,
+CACHEATTR = 98,
 ATOMCTL = 99,
 IBREAKA = 128,
 DBREAKA = 144,
diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index ecd0182..200fb43 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -438,6 +438,24 @@ static unsigned region_attr_to_access(uint32_t attr)
 return access[attr & 0xf];
 }
 
+/*!
+ * Convert cacheattr to PAGE_{READ,WRITE,EXEC} mask.
+ * See ISA, A.2.14 The Cache Attribute Register
+ */
+static unsigned cacheattr_attr_to_access(uint32_t attr)
+{
+static const unsigned access[16] = {
+ [0] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_WT,
+ [1] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WT,
+ [2] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS,
+ [3] =  PAGE_EXEC | PAGE_CACHE_WB,
+ [4] = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_WB,
+[14] = PAGE_READ | PAGE_WRITE | PAGE_CACHE_ISOLATE,
+};
+
+return access[attr & 0xf];
+}
+
 static bool is_access_granted(unsigned access, int is_write)
 {
 switch (is_write) {
@@ -584,7 +602,8 @@ int xtensa_get_physical_addr(CPUXtensaState *env, bool 
update_tlb,
 } else {
 *paddr = vaddr;
 *page_size = TARGET_PAGE_SIZE;
-*access = PAGE_READ | PAGE_WRITE | PAGE_EXEC | PAGE_CACHE_BYPASS;
+*access = cacheattr_attr_to_access(
+env->sregs[CACHEATTR] >> ((vaddr & 0xe000) >> 27));
 return 0;
 }
 }
diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
index 50bf573..45205b8 100644
--- a/target-xtensa/overlay_tool.h
+++ b/target-xtensa/overlay_tool.h
@@ -91,6 +91,7 @@
 XCHAL_OPTION(XCHAL_HAVE_XLT_CACHEATTR, \
 XTENSA_OPTION_REGION_TRANSLATION) | \
 XCHAL_OPTION(XCHAL_HAVE_PTP_MMU, XTENSA_OPTION_MMU) | \
+XCHAL_OPTION(XCHAL_HAVE_CACHEATTR, XTENSA_OPTION_CACHEATTR) | \
 /* Other, TODO */ \
 XCHAL_OPTION(XCHAL_HAVE_WINDOWED, XTENSA_OPTION_WINDOWED_REGISTER) | \
 XCHAL_OPTION(XCHAL_HAVE_DEBUG, XTENSA_OPTION_DEBUG))
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 2ba2360..c246fcb 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -99,6 +99,7 @@ static const char * const sregnames[256] = {
 [ITLBCFG] = "ITLBCFG",
 [DTLBCFG] = "DTLBCFG",
 [IBREAKENABLE] = "IBREAKENABLE",
+[CACHEATTR] = "CACHEATTR",
 [ATOMCTL] = "ATOMCTL",
 [IBREAKA] = "IBREAKA0",
 [IBREAKA + 1] = "IBREAKA1",
-- 
1.7.7.6




Re: [Qemu-devel] [PATCH v2 2/2] ivshmem: use irqfd to interrupt among VMs

2012-12-04 Thread Cam Macdonell
On Tue, Dec 4, 2012 at 4:10 AM, Andrew Jones  wrote:
>
>
> - Original Message -
>> On Thu, Nov 29, 2012 at 1:34 AM, liu ping fan 
>> wrote:
>> > On Thu, Nov 29, 2012 at 12:42 PM, Cam Macdonell
>> >  wrote:
>> >> On Tue, Nov 27, 2012 at 7:53 PM, liu ping fan 
>> >> wrote:
>> >>> On Wed, Nov 28, 2012 at 5:48 AM, Cam Macdonell
>> >>>  wrote:
>>  On Sat, Nov 24, 2012 at 8:51 PM, Liu Ping Fan
>>   wrote:
>> > From: Liu Ping Fan 
>> >
>> > Using irqfd, so we can avoid switch between kernel and user
>> > when
>> > VMs interrupts each other.
>> 
>>  Nice work.  Due to a hardware failure, there will be a small
>>  delay in
>>  me being able to test this.  I'll follow up as soon as I can.
>> 
>> >>> BTW where can I find the latest guest code for test?
>> >>> I got the guest code from
>> >>> git://gitorious.org/nahanni/guest-code.git.
>> >>> But it seems outdated, after fixing the unlocked_ioctl, and vm-id
>> >>> bits
>> >>> position (the codes conflict with ivshmem spec), it works (I have
>> >>> tested for V1).
>> >>
>> >> Hello,
>> >>
>> >> Which device driver are you using?
>> >>
>> > guest-code/kernel_module/standard/kvm_ivshmem.c
>>
>> The uio driver is the recommended one, however if you want to use the
>> kvm_ivshmem one and have it working, then feel free to continue.
>
> If the uio driver is the recommended one, then can you please post it
> to lkml? It should be integrated into drivers/virt with an appropriate
> Kconfig update.
>

Sure.  Should it go under drivers/virt or drivers/uio?  It seems the
uio drivers all get grouped together.

Thanks,
Cam

> Thanks,
> Drew
>
>>
>> I had deleted it form the repo, but some users had based solutions
>> off
>> it, so I added it back.
>>
>> btw, my hardware issue has been resolved, so I'll get to testing your
>> patch soon.
>>
>> Sincerely,
>> Cam
>>
>> >
>> >> Cam
>> >>
>> >>>
>> >>> Regards,
>> >>> Pingfan
>> >>>
>> >
>> > Signed-off-by: Liu Ping Fan 
>> > ---
>> >  hw/ivshmem.c |   54
>> >  +-
>> >  1 files changed, 53 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/hw/ivshmem.c b/hw/ivshmem.c
>> > index 7c8630c..5709e89 100644
>> > --- a/hw/ivshmem.c
>> > +++ b/hw/ivshmem.c
>> > @@ -19,6 +19,7 @@
>> >  #include "hw.h"
>> >  #include "pc.h"
>> >  #include "pci.h"
>> > +#include "msi.h"
>> >  #include "msix.h"
>> >  #include "kvm.h"
>> >  #include "migration.h"
>> > @@ -83,6 +84,7 @@ typedef struct IVShmemState {
>> >  uint32_t vectors;
>> >  uint32_t features;
>> >  EventfdEntry *eventfd_table;
>> > +int *vector_virqs;
>> >
>> >  Error *migration_blocker;
>> >
>> > @@ -625,16 +627,62 @@ static int ivshmem_load(QEMUFile* f, void
>> > *opaque, int version_id)
>> >  return 0;
>> >  }
>> >
>> > +static int ivshmem_vector_use(PCIDevice *dev, unsigned vector,
>> > + MSIMessage msg)
>> > +{
>> > +IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
>> > +int virq;
>> > +EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
>> > +
>> > +virq = kvm_irqchip_add_msi_route(kvm_state, msg);
>> > +if (virq >= 0 && kvm_irqchip_add_irqfd_notifier(kvm_state,
>> > n, virq) >= 0) {
>> > +s->vector_virqs[vector] = virq;
>> > +qemu_chr_add_handlers(s->eventfd_chr[vector], NULL,
>> > NULL, NULL, NULL);
>> > +} else if (virq >= 0) {
>> > +kvm_irqchip_release_virq(kvm_state, virq);
>> > +error_report("ivshmem, can not setup irqfd\n");
>> > +return -1;
>> > +} else {
>> > +error_report("ivshmem, no enough msi route to setup
>> > irqfd\n");
>> > +return -1;
>> > +}
>> > +
>> > +return 0;
>> > +}
>> > +
>> > +static void ivshmem_vector_release(PCIDevice *dev, unsigned
>> > vector)
>> > +{
>> > +IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
>> > +EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
>> > +int virq = s->vector_virqs[vector];
>> > +
>> > +if (s->vector_virqs[vector] >= 0) {
>> > +kvm_irqchip_remove_irqfd_notifier(kvm_state, n, virq);
>> > +kvm_irqchip_release_virq(kvm_state, virq);
>> > +s->vector_virqs[vector] = -1;
>> > +}
>> > +}
>> > +
>> >  static void ivshmem_write_config(PCIDevice *pci_dev, uint32_t
>> >  address,
>> >  uint32_t val, int len)
>> >  {
>> > +bool is_enabled, was_enabled = msi_enabled(pci_dev);
>> > +
>> >  pci_default_write_config(pci_dev, address, val, len);
>> > +is_enabled = msi_enabled(pci_dev);
>> > +if (!was_enabled && is_enabled) {
>> > +msix_set_vector_noti

[Qemu-devel] [PATCH 7/8] target-xtensa: add s32c1i unit tests

2012-12-04 Thread Max Filippov
Signed-off-by: Max Filippov 
---
 tests/tcg/xtensa/Makefile  |1 +
 tests/tcg/xtensa/test_s32c1i.S |   39 +++
 2 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 tests/tcg/xtensa/test_s32c1i.S

diff --git a/tests/tcg/xtensa/Makefile b/tests/tcg/xtensa/Makefile
index 56cfe0f..002fd87 100644
--- a/tests/tcg/xtensa/Makefile
+++ b/tests/tcg/xtensa/Makefile
@@ -42,6 +42,7 @@ endif
 TESTCASES += test_quo.tst
 TESTCASES += test_rem.tst
 TESTCASES += test_rst0.tst
+TESTCASES += test_s32c1i.tst
 TESTCASES += test_sar.tst
 TESTCASES += test_sext.tst
 TESTCASES += test_shift.tst
diff --git a/tests/tcg/xtensa/test_s32c1i.S b/tests/tcg/xtensa/test_s32c1i.S
new file mode 100644
index 000..4536015
--- /dev/null
+++ b/tests/tcg/xtensa/test_s32c1i.S
@@ -0,0 +1,39 @@
+.include "macros.inc"
+
+test_suite s32c1i
+
+test s32c1i_nowrite
+movia2, 1f
+movia3, 1
+wsr a3, scompare1
+movia1, 2
+s32c1i  a1, a2, 0
+assert  ne, a1, a3
+l32ia1, a2, 0
+assert  eqi, a1, 3
+
+.data
+.align 4
+1:
+.word   3
+.text
+test_end
+
+test s32c1i_write
+movia2, 1f
+movia3, 3
+wsr a3, scompare1
+movia1, 2
+s32c1i  a1, a2, 0
+assert  eq, a1, a3
+l32ia1, a2, 0
+assert  eqi, a1, 2
+
+.data
+.align 4
+1:
+.word   3
+.text
+test_end
+
+test_suite_end
-- 
1.7.7.6




[Qemu-devel] [PATCH 8/8] target-xtensa: use movcond where possible

2012-12-04 Thread Max Filippov
Use movcond for all sorts of conditional moves, ABS, CLAMPS, MIN/MAX
opcodes.

Signed-off-by: Max Filippov 
---
 target-xtensa/translate.c |   92 
 1 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 48a22de..7b32123 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -1403,12 +1403,14 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 
 case 1: /*ABS*/
 {
-int label = gen_new_label();
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_T]);
-tcg_gen_brcondi_i32(
-TCG_COND_GE, cpu_R[RRR_R], 0, label);
-tcg_gen_neg_i32(cpu_R[RRR_R], cpu_R[RRR_T]);
-gen_set_label(label);
+TCGv_i32 zero = tcg_const_i32(0);
+TCGv_i32 neg = tcg_temp_new_i32();
+
+tcg_gen_neg_i32(neg, cpu_R[RRR_T]);
+tcg_gen_movcond_i32(TCG_COND_GE, cpu_R[RRR_R],
+cpu_R[RRR_T], zero, cpu_R[RRR_T], neg);
+tcg_temp_free(neg);
+tcg_temp_free(zero);
 }
 break;
 
@@ -1755,22 +1757,20 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 {
 TCGv_i32 tmp1 = tcg_temp_new_i32();
 TCGv_i32 tmp2 = tcg_temp_new_i32();
-int label = gen_new_label();
+TCGv_i32 zero = tcg_const_i32(0);
 
 tcg_gen_sari_i32(tmp1, cpu_R[RRR_S], 24 - RRR_T);
 tcg_gen_xor_i32(tmp2, tmp1, cpu_R[RRR_S]);
 tcg_gen_andi_i32(tmp2, tmp2, 0x << (RRR_T + 7));
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]);
-tcg_gen_brcondi_i32(TCG_COND_EQ, tmp2, 0, label);
 
 tcg_gen_sari_i32(tmp1, cpu_R[RRR_S], 31);
-tcg_gen_xori_i32(cpu_R[RRR_R], tmp1,
-0x >> (25 - RRR_T));
-
-gen_set_label(label);
+tcg_gen_xori_i32(tmp1, tmp1, 0x >> (25 - RRR_T));
 
+tcg_gen_movcond_i32(TCG_COND_EQ, cpu_R[RRR_R], tmp2, zero,
+cpu_R[RRR_S], tmp1);
 tcg_temp_free(tmp1);
 tcg_temp_free(tmp2);
+tcg_temp_free(zero);
 }
 break;
 
@@ -1787,19 +1787,9 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 TCG_COND_LEU,
 TCG_COND_GEU
 };
-int label = gen_new_label();
-
-if (RRR_R != RRR_T) {
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]);
-tcg_gen_brcond_i32(cond[OP2 - 4],
-cpu_R[RRR_S], cpu_R[RRR_T], label);
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_T]);
-} else {
-tcg_gen_brcond_i32(cond[OP2 - 4],
-cpu_R[RRR_T], cpu_R[RRR_S], label);
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]);
-}
-gen_set_label(label);
+tcg_gen_movcond_i32(cond[OP2 - 4], cpu_R[RRR_R],
+cpu_R[RRR_S], cpu_R[RRR_T],
+cpu_R[RRR_S], cpu_R[RRR_T]);
 }
 break;
 
@@ -1810,15 +1800,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 gen_window_check3(dc, RRR_R, RRR_S, RRR_T);
 {
 static const TCGCond cond[] = {
-TCG_COND_NE,
 TCG_COND_EQ,
+TCG_COND_NE,
+TCG_COND_LT,
 TCG_COND_GE,
-TCG_COND_LT
 };
-int label = gen_new_label();
-tcg_gen_brcondi_i32(cond[OP2 - 8], cpu_R[RRR_T], 0, label);
-tcg_gen_mov_i32(cpu_R[RRR_R], cpu_R[RRR_S]);
-gen_set_label(label);
+TCGv_i32 zero = tcg_const_i32(0);
+
+tcg_gen_movcond_i32(cond[OP2 - 8], cpu_R[RRR_R],
+cpu_R[RRR_T], zero, cpu_R[RRR_S], cpu_R[RRR_R]);
+tcg_temp_free(zero);
 }
 break;
 
@@ -1827,16 +1818,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 HAS_OPTION(XTENSA_OPTION_BOOLEAN);
 gen_window_check2(dc, RRR_R, RRR_S);
 {
-  

[Qemu-devel] [PATCH 0/8] xtensa patch queue

2012-12-04 Thread Max Filippov
Hi.

This is my current patch queue for xtensa:
- add support for a number of Special Registers: ATOMCTL, CACHEATTR, MISC;
- raise exceptions on access to unconfigured SRs/invalid access to configured 
SRs;
- add unit tests for SR access and for s32c1i opcode;
- use movcond to re-implement some opcodes more efficiently.

Please review/apply.

Max Filippov (8):
  target-xtensa: implement ATOMCTL SR
  target-xtensa: implement CACHEATTR SR
  target-xtensa: restrict available SRs by enabled options
  target-xtensa: better control rsr/wsr/xsr access to SRs
  target-xtensa: implement MISC SR
  target-xtensa: add SR accessibility unit tests
  target-xtensa: add s32c1i unit tests
  target-xtensa: use movcond where possible

 target-xtensa/cpu.c|3 +
 target-xtensa/cpu.h|   14 ++
 target-xtensa/helper.c |   75 +++--
 target-xtensa/helper.h |1 +
 target-xtensa/op_helper.c  |   57 ++
 target-xtensa/overlay_tool.h   |   12 ++-
 target-xtensa/translate.c  |  367 ++--
 tests/tcg/xtensa/Makefile  |2 +
 tests/tcg/xtensa/macros.inc|2 +-
 tests/tcg/xtensa/test_s32c1i.S |   39 +
 tests/tcg/xtensa/test_sr.S |   90 ++
 11 files changed, 484 insertions(+), 178 deletions(-)
 create mode 100644 tests/tcg/xtensa/test_s32c1i.S
 create mode 100644 tests/tcg/xtensa/test_sr.S

-- 
1.7.7.6




[Qemu-devel] [Bug 955379] Re: cmake hangs with qemu-arm-static

2012-12-04 Thread Peter Maydell
Yes. You can never shut the window completely trying to do it that way,
which is why you need fix the problem properly instead.

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

Title:
  cmake hangs with qemu-arm-static

Status in QEMU:
  Confirmed
Status in Linaro QEMU:
  Confirmed
Status in “qemu-linaro” package in Ubuntu:
  Confirmed

Bug description:
  I'm using git commit 3e7ecd976b06f... configured with --target-list
  =arm-linux-user --static in a chroot environment to compile some
  things. I ran into this problem with both pcl and opencv-2.3.1. cmake
  consistently freezes at some point during its execution, though in a
  different spot each time, usually during a step when it's searching
  for some libraries. For instance, pcl most commonly stops after:

  [snip]
  -- Boost version: 1.46.1
  -- Found the following Boost libraries:
  --   system
  --   filesystem
  --   thread
  --   date_time
  -- checking for module 'eigen3'
  --   found eigen3, version 3.0.1

  which is perplexing because it freezes after finding what it wants,
  not during the search. When it does get past that point, it does so
  almost immediately but freezes somewhere else.

  I'm using 64-bit Ubuntu 11.10 with kernel release 3.0.0-16-generic
  with an Intel i5.

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



[Qemu-devel] [PATCH 0/3] target-i386:make breakpoint more cleaner

2012-12-04 Thread li guang
try to make breakpoint related functions
more cleaner.
originally, these functions implicit use
0,1,2,3 as name of breakpoint, really
hard to understand, so name them readable,
and also refactor the statement to
check these types for clean reason.


 target-i386/cpu.h |11 ++--
 target-i386/helper.c  |70 --
 target-i386/machine.c |2  +-
 target-i386/misc_helper.c |4  +-
 target-i386/seg_helper.c  |6  ++--
 5 files changed, 60 insertions(+), 33 deletions(-)

-- 
regards!
li guang




Re: [Qemu-devel] [PATCH 08/13] pseries: Update SLOF for NVRAM support

2012-12-04 Thread Alexander Graf

On 04.12.2012, at 14:20, Erlon Cruz wrote:

> 
> 
> On Tue, Dec 4, 2012 at 12:42 AM, David Gibson  
> wrote:
> Now that we have implemented PAPR compatible NVRAM interfaces in qemu, this
> updates the SLOF firmware to actually initialize and use the NVRAM as a
> PAPR guest firmware is expected to do.
> 
> This SLOF update also includes an ugly but useful workaround for a bug in
> the SLES11 installer which caused it to fail under KVM.
> 
> 
> I had a problem when installing SLES, when the installer et at 23%, a get:
> 
> Installation of package ./suse/ppc64/vim-base-7.2-8.15.2.ppc64.rpm failed.
> Subprocess failed. Error: RPM failed: error: 
> %post(vim-base-7.2-8.15.2.ppc64.rpm)

Could you please post

  * the exact command line you were using
  * details about your host: architecture, OS, kernel version
  * is this using KVM or TCG?
  * what SLES exactly are you using here?
  * the exact QEMU version


Alex



Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

2012-12-04 Thread Gerd Hoffmann
  Hi,

> Gerd,
> 
> Is there any documentation out there on how to tell QEMU on command
> line which EHCI you want your usb-storage to attach to?

docs/usb2.txt has some examples, although those are pc-centric where the
ehci is created on the command line.  The problem with zynx is that both
ehci controllers get the same (default) name so picking one by name
doesn't work.

Guess we haver to figure a way to explicitly name those devices (so the
busses are named too), especially in case a board has two identical
ones.  Maybe sysbus_create_simple needs an additional argument or a
sysbus_create_simple_with_id variant.

cheers,
  Gerd




Re: [Qemu-devel] [RFC 1/8] move -I$(SRC_PATH)/include compiler flag to Makefile.objs

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 05:03:36PM +0100, Andreas Färber wrote:
> Am 04.12.2012 14:19, schrieb Eduardo Habkost:
> > The flag is necessary for code that doesn't use the variables from
> > Makefile (but use Makefile.objs), like libcacard/ and stubs/.
> > 
> > Signed-off-by: Eduardo Habkost 
> 
> I don't quite understand the rationale of this patch.
> libcacard/ and stubs/ shouldn't need vl.o, do they? The CFLAGS move
> makes more sense to me.

The change is not about vl.o, is about having having the header files from
include/qemu available when building libcacard and libqemustub.

I had to do that because now some files included by files in libcacard/ and
stubs/ end up including header files from include/qemu. e.g.:

 lt CC stubs/sysbus.lo
In file included from /home/ehabkost/pessoal/proj/virt/qemu/stubs/sysbus.c:1:0:
/home/ehabkost/pessoal/proj/virt/qemu/hw/qdev-core.h:7:25: fatal error: 
qemu/object.h: No such file or directory
compilation terminated.
make[1]: *** [stubs/sysbus.lo] Error 1
make: *** [subdir-libcacard] Error 2

I remember seeing a similar error when building libcacard while working in this
series, but now I can't trigger it anymore.


> 
> Paolo, can you take a look please?
> 
> Thanks,
> Andreas
> 
> > ---
> >  Makefile  |  1 -
> >  Makefile.objs | 15 +--
> >  2 files changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 9ecbcbb..739d9cd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -145,7 +145,6 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += 
> > $(FMOD_CFLAGS)
> >  
> >  QEMU_CFLAGS+=$(CURL_CFLAGS)
> >  
> > -QEMU_CFLAGS += -I$(SRC_PATH)/include
> >  
> >  ui/cocoa.o: ui/cocoa.m
> >  
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 3c7abca..0a0a33a 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -1,4 +1,13 @@
> >  ###
> > +# general compiler flags
> > +
> > +QEMU_CFLAGS += $(GLIB_CFLAGS)
> > +QEMU_CFLAGS += -I$(SRC_PATH)/include
> > +
> > +vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> > +vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> > +
> > +###
> >  # Stub library, linked in tools
> >  stub-obj-y = stubs/
> >  
> > @@ -236,12 +245,6 @@ universal-obj-y += $(qapi-obj-y)
> >  qga-obj-y = qga/ qemu-ga.o module.o qemu-tool.o
> >  qga-obj-$(CONFIG_POSIX) += qemu-sockets.o qemu-option.o
> >  
> > -vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> > -
> > -vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> > -
> > -QEMU_CFLAGS+=$(GLIB_CFLAGS)
> > -
> >  nested-vars += \
> > stub-obj-y \
> > qga-obj-y \
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo



[Qemu-devel] [PATCH 4/4] exec: refactor cpu_restore_state

2012-12-04 Thread Blue Swirl
Refactor common code around calls to cpu_restore_state().

tb_find_pc() has now no external users, make it static.

Signed-off-by: Blue Swirl 
---
 exec-all.h|6 ++
 hw/kvmvapic.c |4 +---
 target-alpha/helper.c |   14 +++---
 target-alpha/mem_helper.c |8 ++--
 target-arm/op_helper.c|8 +---
 target-cris/op_helper.c   |8 +---
 target-i386/helper.c  |5 +
 target-i386/mem_helper.c  |8 +---
 target-lm32/op_helper.c   |8 +---
 target-m68k/op_helper.c   |8 +---
 target-microblaze/op_helper.c |8 +---
 target-mips/op_helper.c   |8 +---
 target-openrisc/mmu_helper.c  |   10 +-
 target-ppc/mem_helper.c   |8 +---
 target-s390x/mem_helper.c |8 +---
 target-sh4/op_helper.c|   23 ++-
 target-sparc/cpu.h|1 -
 target-sparc/helper.c |   12 ++--
 target-sparc/ldst_helper.c|   24 ++--
 target-unicore32/op_helper.c  |9 +
 target-xtensa/op_helper.c |   14 ++
 translate-all.c   |   27 ---
 user-exec.c   |8 +---
 23 files changed, 65 insertions(+), 172 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 21aacda..b6d8279 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -84,8 +84,8 @@ void restore_state_to_opc(CPUArchState *env, struct 
TranslationBlock *tb,
 void cpu_gen_init(void);
 int cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb,
  int *gen_code_size_ptr);
-int cpu_restore_state(struct TranslationBlock *tb,
-  CPUArchState *env, uintptr_t searched_pc);
+bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
+
 void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
 void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUArchState *env, 
@@ -279,8 +279,6 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
 }
 }
 
-TranslationBlock *tb_find_pc(uintptr_t pc_ptr);
-
 #include "qemu-lock.h"
 
 extern spinlock_t tb_lock;
diff --git a/hw/kvmvapic.c b/hw/kvmvapic.c
index e04c401..60c8fc4 100644
--- a/hw/kvmvapic.c
+++ b/hw/kvmvapic.c
@@ -387,7 +387,6 @@ static void patch_instruction(VAPICROMState *s, CPUX86State 
*env, target_ulong i
 VAPICHandlers *handlers;
 uint8_t opcode[2];
 uint32_t imm32;
-TranslationBlock *current_tb;
 target_ulong current_pc = 0;
 target_ulong current_cs_base = 0;
 int current_flags = 0;
@@ -399,8 +398,7 @@ static void patch_instruction(VAPICROMState *s, CPUX86State 
*env, target_ulong i
 }
 
 if (!kvm_enabled()) {
-current_tb = tb_find_pc(env->mem_io_pc);
-cpu_restore_state(current_tb, env, env->mem_io_pc);
+cpu_restore_state(env, env->mem_io_pc);
 cpu_get_tb_cpu_state(env, ¤t_pc, ¤t_cs_base,
  ¤t_flags);
 }
diff --git a/target-alpha/helper.c b/target-alpha/helper.c
index d9d7f75..2430f70 100644
--- a/target-alpha/helper.c
+++ b/target-alpha/helper.c
@@ -494,16 +494,6 @@ void cpu_dump_state (CPUAlphaState *env, FILE *f, 
fprintf_function cpu_fprintf,
 cpu_fprintf(f, "\n");
 }
 
-void do_restore_state(CPUAlphaState *env, uintptr_t retaddr)
-{
-if (retaddr) {
-TranslationBlock *tb = tb_find_pc(retaddr);
-if (tb) {
-cpu_restore_state(tb, env, retaddr);
-}
-}
-}
-
 /* This should only be called from translate, via gen_excp.
We expect that ENV->PC has already been updated.  */
 void QEMU_NORETURN helper_excp(CPUAlphaState *env, int excp, int error)
@@ -519,7 +509,9 @@ void QEMU_NORETURN dynamic_excp(CPUAlphaState *env, 
uintptr_t retaddr,
 {
 env->exception_index = excp;
 env->error_code = error;
-do_restore_state(env, retaddr);
+if (retaddr) {
+cpu_restore_state(env, retaddr);
+}
 cpu_loop_exit(env);
 }
 
diff --git a/target-alpha/mem_helper.c b/target-alpha/mem_helper.c
index 617836c..64b33f6 100644
--- a/target-alpha/mem_helper.c
+++ b/target-alpha/mem_helper.c
@@ -94,7 +94,9 @@ static void do_unaligned_access(CPUAlphaState *env, 
target_ulong addr,
 uint64_t pc;
 uint32_t insn;
 
-do_restore_state(env, retaddr);
+if (retaddr) {
+cpu_restore_state(env, retaddr);
+}
 
 pc = env->pc;
 insn = cpu_ldl_code(env, pc);
@@ -143,7 +145,9 @@ void tlb_fill(CPUAlphaState *env, target_ulong addr, int 
is_write,
 
 ret = cpu_alpha_handle_mmu_fault(env, addr, is_write, mmu_idx);
 if (unlikely(ret != 0)) {
-do_restore_state(env, retaddr);
+if (retaddr) {
+cpu_restore_state(env, retaddr);
+}
 /* Exception index and error code are already set */
 cpu_loop_exit(env);
 }
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
ind

Re: [Qemu-devel] [PATCH V6 00/10] replace QEMUOptionParameter with QemuOpts parser

2012-12-04 Thread Dong Xu Wang
On Mon, Dec 3, 2012 at 8:42 PM, Stefan Hajnoczi  wrote:
> On Fri, Nov 23, 2012 at 8:47 AM, Dong Xu Wang
>  wrote:
>> Patch 1-3 are from Luiz, added Markus's comments, discussion could be found 
>> here:
>> http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html
>> Patch 3 was changed according Paolo's comments.
>>
>> Patch 4-5: because qemu_opts_create can not fail while id is null, so create
>> function qemu_opts_create_nofail and use it.
>>
>> Patch 6: create function qemu_opt_set_number, like qemu_opt_set_bool.
>>
>> Patch 7: add def_value and use it in qemu_opts_print.
>>
>> Patch 8: Create functions to pair with QEMUOptionParameter parser.
>>
>> Patch 9: Use QemuOpts parser in Block.
>>
>> Patch 10: Remove QEMUOptionParameter parser related code.
>>
>> v5->v6:
>> 1) allocate enough space in append_opts_list function.
>> 2) judge if opts == NULL in block layer create functions.
>> 3) use bdrv_create_file(filename, NULL) in qcow_create funtion.
>> 4) made more readable while using qemu_opt_get_number funtion.
>>
>> v4->v5:
>> 1) Rewrite qemu_opts_create_nofail function based on Peter Maydell's 
>> comments.
>> 2) Use g_strdup_printf in qemu_opt_set_number.
>> 3) Rewrite qemu_opts_print.
>> 4) .bdrv_create_options returns pointer directly. Fix a bug about 
>> "encryption".
>> 5) Check qemu_opt_get_number in raw-posix.c.
>>
>> v3->v4:
>> 1) Rebased to the newest source tree.
>> 2) Remove redundant "#include "block-cache.h"
>> 3) Other small changes.
>>
>> v2->v3:
>> 1) rewrite qemu_opt_set_bool and qemu_opt_set_number according Paolo's 
>> coments.
>> 2) split patches to make review easier.
>>
>> v1->v2:
>> 1) add Luiz's patches.
>> 2) create qemu_opt_set_number() and qemu_opts_create_nofail() functions.
>> 3) add QemuOptsList map to drivers.
>> 4) use original opts parser, not creating new ones.
>> 5) fix other bugs.
>>
>> Dong Xu Wang (10):
>>   qemu-option: opt_set(): split it up into more functions
>>   qemu-option: qemu_opts_validate(): fix duplicated code
>>   qemu-option: qemu_opt_set_bool(): fix code duplication
>>   introduce qemu_opts_create_nofail function
>>   use qemu_opts_create_nofail
>>   create new function: qemu_opt_set_number
>>   add def_print_str and use it in qemu_opts_print.
>>   Create four opts list related functions
>>   Use QemuOpts support in block layer
>>   remove QEMUOptionParameter related functions and struct
>>
>>  block.c   |   91 +-
>>  block.h   |8 +-
>>  block/cow.c   |   46 +++---
>>  block/qcow.c  |   60 +++---
>>  block/qcow2.c |  171 +-
>>  block/qed.c   |   86 +-
>>  block/raw-posix.c |   65 
>>  block/raw.c   |   30 ++--
>>  block/sheepdog.c  |   75 
>>  block/vdi.c   |   68 
>>  block/vmdk.c  |   74 
>>  block/vpc.c   |   65 ---
>>  block/vvfat.c |   11 +-
>>  block_int.h   |6 +-
>>  blockdev.c|2 +-
>>  hw/watchdog.c |2 +-
>>  qemu-config.c |4 +-
>>  qemu-img.c|   63 +++
>>  qemu-option.c |  512 
>> ++---
>>  qemu-option.h |   39 +
>>  qemu-sockets.c|   16 +-
>>  vl.c  |   12 +-
>>  22 files changed, 658 insertions(+), 848 deletions(-)
>
> This is close to being merged.  I have posted a few small remaining comments.
>
> Stefan
>
Thank you stefan, will sent v7 soon.



Re: [Qemu-devel] [PATCH 1/3] target-i386:define name of breakpoint bit in dr7

2012-12-04 Thread li guang
在 2012-12-04二的 12:49 +,Peter Maydell写道:
> On 4 December 2012 08:11, liguang  wrote:
> > Signed-off-by: liguang 
> > ---
> >  target-i386/cpu.h |7 +++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 90ef1ff..9abec3e 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -231,6 +231,13 @@
> >  #define DR7_TYPE_SHIFT  16
> >  #define DR7_LEN_SHIFT   18
> >  #define DR7_FIXED_1 0x0400
> > +#define DR7_LOCAL_BP_MASK   0x55
> > +#define DR7_MAX_BP  4
> > +#define DR7_BP_INST 0x0
> > +#define DR7_DATA_WR 0x1
> > +#define DR7_IO_RW   0x2
> > +#define DR7_DATA_RW 0x3
> 
> I still think these last four should be DR7_TYPE_BP_INST etc to
> indicate that they're values for the TYPE field, not direct
> specifications of bits in DR7.

hmm, is it necessary?
you know, the use of these names
is after calling 'hw_breakpoint_type'
function, so,
it's obvious for dr7's type field.

> 
> -- PMM

-- 
regards!
li guang




[Qemu-devel] [BUG] QEMU crashes when 64bit BAR is present

2012-12-04 Thread Alexey Korolev
Hi all,
I had qemu 1.2.0 crash when using ivshmem driver with 64bit PCI support 
enabled. The qemu process is terminated at a very early stage of
Linux boot up. Here is the qemu command line:

LC_ALL=C 
PATH=/usr/kerberos/sbin:/usr/kerberos/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/root/bin
 HOME=/home/user
USER=user LOGNAME=user QEMU_AUDIO_DRV=none /usr/bin/qemu -M pc-0.11 -enable-kvm 
-m 4096 -smp 1,sockets=1,cores=1,threads=1 -name Cent5 -uuid
59342423-be7a-0f83-b9ac-35a42e521d99 -nodefconfig -nodefaults -chardev
socket,id=charmonitor,path=/var/lib/libvirt/qemu/Cent5.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=readline -rtc base=utc
-boot order=cd,menu=on -drive 
file=/home/akorolev/Cent54.img,if=none,id=drive-ide0-0-0,format=raw -device
ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive 
if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device
ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -chardev 
file,id=charserial0,path=/home/akorolev/Cent5.5.log -device
isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -k en-us -vga 
cirrus -device
ivshmem,size=128M,pci64=1,shm,bus=pci.0,multifunction=on,addr=0x5.0x0 -device
virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x4.0x0

And qemu error output is:
qemu: /home/akorolev/qemu-kvm/exec.c:2255: register_subpage: Assertion 
`existing->mr->subpage || existing->mr == &io_mem_unassigned' failed.

Guest OS is Centos 5.5 and log is pretty boring, as qemu crashes before Linux 
can report an issue.

Note: The only tweak I've made to qemu is changing PCI bar flag to 
PCI_ADDRESS_MEM_TYPE_64 in ivshmem driver

I guess the issue is related to this: 
http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03189.html
At that time /it was found out that ivshmem bar is split by/ /the hpet region/

Could you please have a look at this?
I'm willing to provide additional info if you need it.

Thanks
Alexey




[Qemu-devel] [PATCH 1/4] exec: fix coding style

2012-12-04 Thread Blue Swirl
Fix coding style in areas to be moved by later patches.

Signed-off-by: Blue Swirl 
---
 exec.c |  178 +++
 1 files changed, 110 insertions(+), 68 deletions(-)

diff --git a/exec.c b/exec.c
index 8435de0..6efd93e 100644
--- a/exec.c
+++ b/exec.c
@@ -79,6 +79,7 @@
 
 #define SMC_BITMAP_USE_THRESHOLD 10
 
+/* Code generation and translation blocks */
 static TranslationBlock *tbs;
 static int code_gen_max_blocks;
 TranslationBlock *tb_phys_hash[CODE_GEN_PHYS_HASH_SIZE];
@@ -207,21 +208,20 @@ static inline void map_exec(void *addr, long size)
 DWORD old_protect;
 VirtualProtect(addr, size,
PAGE_EXECUTE_READWRITE, &old_protect);
-
 }
 #else
 static inline void map_exec(void *addr, long size)
 {
 unsigned long start, end, page_size;
-
+
 page_size = getpagesize();
 start = (unsigned long)addr;
 start &= ~(page_size - 1);
-
+
 end = (unsigned long)addr + size;
 end += page_size - 1;
 end &= ~(page_size - 1);
-
+
 mprotect((void *)start, end - start,
  PROT_READ | PROT_WRITE | PROT_EXEC);
 }
@@ -241,10 +241,12 @@ static void page_init(void)
 #else
 qemu_real_host_page_size = getpagesize();
 #endif
-if (qemu_host_page_size == 0)
+if (qemu_host_page_size == 0) {
 qemu_host_page_size = qemu_real_host_page_size;
-if (qemu_host_page_size < TARGET_PAGE_SIZE)
+}
+if (qemu_host_page_size < TARGET_PAGE_SIZE) {
 qemu_host_page_size = TARGET_PAGE_SIZE;
+}
 qemu_host_page_mask = ~(qemu_host_page_size - 1);
 
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
@@ -291,7 +293,7 @@ static void page_init(void)
 unsigned long startaddr, endaddr;
 int n;
 
-n = fscanf (f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr);
+n = fscanf(f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr);
 
 if (n == 2 && h2g_valid(startaddr)) {
 startaddr = h2g(startaddr) & TARGET_PAGE_MASK;
@@ -591,6 +593,7 @@ static inline void *alloc_code_gen_buffer(void)
 static inline void *alloc_code_gen_buffer(void)
 {
 void *buf = g_malloc(code_gen_buffer_size);
+
 if (buf) {
 map_exec(buf, code_gen_buffer_size);
 }
@@ -735,8 +738,9 @@ static TranslationBlock *tb_alloc(target_ulong pc)
 TranslationBlock *tb;
 
 if (nb_tbs >= code_gen_max_blocks ||
-(code_gen_ptr - code_gen_buffer) >= code_gen_buffer_max_size)
+(code_gen_ptr - code_gen_buffer) >= code_gen_buffer_max_size) {
 return NULL;
+}
 tb = &tbs[nb_tbs++];
 tb->pc = pc;
 tb->cflags = 0;
@@ -764,8 +768,7 @@ static inline void invalidate_page_bitmap(PageDesc *p)
 }
 
 /* Set to NULL all the 'first_tb' fields in all PageDescs. */
-
-static void page_flush_tb_1 (int level, void **lp)
+static void page_flush_tb_1(int level, void **lp)
 {
 int i;
 
@@ -774,14 +777,16 @@ static void page_flush_tb_1 (int level, void **lp)
 }
 if (level == 0) {
 PageDesc *pd = *lp;
+
 for (i = 0; i < L2_SIZE; ++i) {
 pd[i].first_tb = NULL;
 invalidate_page_bitmap(pd + i);
 }
 } else {
 void **pp = *lp;
+
 for (i = 0; i < L2_SIZE; ++i) {
-page_flush_tb_1 (level - 1, pp + i);
+page_flush_tb_1(level - 1, pp + i);
 }
 }
 }
@@ -789,6 +794,7 @@ static void page_flush_tb_1 (int level, void **lp)
 static void page_flush_tb(void)
 {
 int i;
+
 for (i = 0; i < V_L1_SIZE; i++) {
 page_flush_tb_1(V_L1_SHIFT / L2_BITS - 1, l1_map + i);
 }
@@ -799,22 +805,24 @@ static void page_flush_tb(void)
 void tb_flush(CPUArchState *env1)
 {
 CPUArchState *env;
+
 #if defined(DEBUG_FLUSH)
 printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
(unsigned long)(code_gen_ptr - code_gen_buffer),
nb_tbs, nb_tbs > 0 ?
((unsigned long)(code_gen_ptr - code_gen_buffer)) / nb_tbs : 0);
 #endif
-if ((unsigned long)(code_gen_ptr - code_gen_buffer) > code_gen_buffer_size)
+if ((unsigned long)(code_gen_ptr - code_gen_buffer)
+> code_gen_buffer_size) {
 cpu_abort(env1, "Internal error: code buffer overflow\n");
-
+}
 nb_tbs = 0;
 
-for(env = first_cpu; env != NULL; env = env->next_cpu) {
-memset (env->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof (void *));
+for (env = first_cpu; env != NULL; env = env->next_cpu) {
+memset(env->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof(void *));
 }
 
-memset (tb_phys_hash, 0, CODE_GEN_PHYS_HASH_SIZE * sizeof (void *));
+memset(tb_phys_hash, 0, CODE_GEN_PHYS_HASH_SIZE * sizeof(void *));
 page_flush_tb();
 
 code_gen_ptr = code_gen_buffer;
@@ -829,9 +837,10 @@ static void tb_invalidate_check(target_ulong address)
 {
 TranslationBlock *tb;
 int i;
+
 address &= TARGET_PAGE_MASK;
-for(i = 0;i < CODE_GEN_PHYS_HASH_SIZE

Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type

2012-12-04 Thread Peter Maydell
On 4 December 2012 08:11, liguang  wrote:
> Signed-off-by: liguang 
> ---
>  target-i386/cpu.h |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 9abec3e..8ca25c8 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -996,9 +996,9 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, 
> target_ulong addr,
>  #define cpu_handle_mmu_fault cpu_x86_handle_mmu_fault
>  void cpu_x86_set_a20(CPUX86State *env, int a20_state);
>
> -static inline int hw_breakpoint_enabled(unsigned long dr7, int index)
> +static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
>  {
> -return (dr7 >> (index * 2)) & 3;
> +return !!((dr7 >> (index * 2)) & 3);
>  }
>
>  static inline int hw_breakpoint_type(unsigned long dr7, int index)

Doesn't this break the use of this function in target-i386/seg_helper.c:

  if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {

which specifically wants to determine whether the breakpoint is
enabled only locally?

-- PMM



Re: [Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function

2012-12-04 Thread li guang
在 2012-12-04二的 18:51 +,Blue Swirl写道:
> On Tue, Dec 4, 2012 at 8:11 AM, liguang  wrote:
> > Signed-off-by: liguang 
> > ---
> >  target-i386/helper.c  |   70 
> > +
> >  target-i386/machine.c |2 +-
> >  target-i386/misc_helper.c |4 +-
> >  target-i386/seg_helper.c  |6 ++--
> >  4 files changed, 51 insertions(+), 31 deletions(-)
> >
> > diff --git a/target-i386/helper.c b/target-i386/helper.c
> > index bf206cf..28307a1 100644
> > --- a/target-i386/helper.c
> > +++ b/target-i386/helper.c
> > @@ -966,30 +966,31 @@ hwaddr cpu_get_phys_page_debug(CPUX86State *env, 
> > target_ulong addr)
> >
> >  void hw_breakpoint_insert(CPUX86State *env, int index)
> >  {
> > -int type, err = 0;
> > +int type = 0, err = 0;
> >
> >  switch (hw_breakpoint_type(env->dr[7], index)) {
> > -case 0:
> > -if (hw_breakpoint_enabled(env->dr[7], index))
> > +case DR7_BP_INST:
> > +if (hw_breakpoint_enabled(env->dr[7], index)) {
> >  err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
> >  &env->cpu_breakpoint[index]);
> > +}
> >  break;
> > -case 1:
> > +case DR7_DATA_WR:
> >  type = BP_CPU | BP_MEM_WRITE;
> > -goto insert_wp;
> > -case 2:
> > - /* No support for I/O watchpoints yet */
> > -break;
> > -case 3:
> 
> Missing 'break'.

yes, will fix, thanks!

> 
> > +case DR7_DATA_RW:
> >  type = BP_CPU | BP_MEM_ACCESS;
> > -insert_wp:
> > +   case DR7_IO_RW:
> > + /* No support for I/O watchpoints yet */
> > + break;
> > +}
> > +   if (type) {
> >  err = cpu_watchpoint_insert(env, env->dr[index],
> >  hw_breakpoint_len(env->dr[7], index),
> >  type, &env->cpu_watchpoint[index]);
> > -break;
> >  }
> > -if (err)
> > +if (err) {
> >  env->cpu_breakpoint[index] = NULL;
> > +}
> >  }
> >
> >  void hw_breakpoint_remove(CPUX86State *env, int index)
> > @@ -997,15 +998,16 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
> >  if (!env->cpu_breakpoint[index])
> >  return;
> >  switch (hw_breakpoint_type(env->dr[7], index)) {
> > -case 0:
> > -if (hw_breakpoint_enabled(env->dr[7], index))
> > +case DR7_BP_INST:
> > +if (hw_breakpoint_enabled(env->dr[7], index)) {
> >  cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
> > +}
> >  break;
> > -case 1:
> > -case 3:
> > +case DR7_DATA_RW:
> > +case DR7_DATA_WR:
> >  cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
> >  break;
> > -case 2:
> > +case DR7_IO_RW:
> >  /* No support for I/O watchpoints yet */
> >  break;
> >  }
> > @@ -1014,22 +1016,40 @@ void hw_breakpoint_remove(CPUX86State *env, int 
> > index)
> >  int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
> >  {
> >  target_ulong dr6;
> > -int reg, type;
> > +int index;
> >  int hit_enabled = 0;
> > +bool bp_match = false;
> > +bool wp_match = false;
> >
> >  dr6 = env->dr[6] & ~0xf;
> > -for (reg = 0; reg < 4; reg++) {
> > -type = hw_breakpoint_type(env->dr[7], reg);
> > -if ((type == 0 && env->dr[reg] == env->eip) ||
> > -((type & 1) && env->cpu_watchpoint[reg] &&
> > - (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
> > -dr6 |= 1 << reg;
> > -if (hw_breakpoint_enabled(env->dr[7], reg))
> > +   for (index = 0; index < DR7_MAX_BP; index++) {
> > +switch (hw_breakpoint_type(env->dr[7], index)) {
> > +case DR7_BP_INST:
> > +if (env->dr[index] == env->eip) {
> > +bp_match = true;
> > +}
> > +break;
> > +case DR7_DATA_WR:
> > +case DR7_DATA_RW:
> > +if (env->cpu_watchpoint[index] &&
> > +env->cpu_watchpoint[index]->flags & BP_WATCHPOINT_HIT) {
> > +wp_match = true;
> > +}
> 
> Also here.
> 

No, just fall through.

> > +case DR7_IO_RW:
> > +break;
> > +}
> > +if (bp_match || wp_match) {
> > +dr6 |= 1 << index;
> > +if (hw_breakpoint_enabled(env->dr[7], index)) {
> >  hit_enabled = 1;
> > +}
> > +bp_match = false;
> > +wp_match = false;
> >  }
> >  }
> >  if (hit_enabled || force_dr6_update)
> >  env->dr[6] = dr6;
> > +
> >  return hit_enabled;
> >  }
> >
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index 4771508..a4b1a1e 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -265,7 +265,7 @@ static int cpu_post_load(void *opaque, int version_id)
> >
> >  cpu_breakpoint_

Re: [Qemu-devel] [PATCH 2/3] target-i386:make hw_breakpoint_enabled return bool type

2012-12-04 Thread li guang
在 2012-12-04二的 11:26 +,Peter Maydell写道:
> On 4 December 2012 11:11, Jan Kiszka  wrote:
> > On 2012-12-04 11:23, Peter Maydell wrote:
> >> Doesn't this break the use of this function in target-i386/seg_helper.c:
> >>
> >>   if (hw_breakpoint_enabled(env->dr[7], i) == 0x1) {
> >>
> >> which specifically wants to determine whether the breakpoint is
> >> enabled only locally?

 It was changed to 'if (hw_breakpoint_enabled(env->dr[7], i)) {'
 in patch 3/3

> >
> > It does. And that also indicates the function is misnamed. Something
> > like hw_breakpoint_state might be better.
> 

misnamed? I think hw_breakpoint_enabled is ask whether breakpoint
 
is enabled or not, so it's almost suitable.

> And/or we could just refactor the code in seg_helper.c, which is
> a nasty mix of direct access to dr[7] and using the hw_breakpoint_*
> functions.
> 
> -- PMM

-- 
regards!
li guang




[Qemu-devel] [PATCH 0/4] exec.c refactoring

2012-12-04 Thread Blue Swirl
Refactor TranslationBlock handling out of exec.c.

This could also help make TCG more optional or
add other code generators.

Blue Swirl (4):
  exec: fix coding style
  exec: extract TB watchpoint check
  exec: move TB handling to translate-all.c
  exec: refactor cpu_restore_state

 exec-all.h|6 +-
 exec.c| 1665 +---
 hw/kvmvapic.c |4 +-
 target-alpha/helper.c |   14 +-
 target-alpha/mem_helper.c |8 +-
 target-arm/op_helper.c|8 +-
 target-cris/op_helper.c   |8 +-
 target-i386/helper.c  |5 +-
 target-i386/mem_helper.c  |8 +-
 target-lm32/op_helper.c   |8 +-
 target-m68k/op_helper.c   |8 +-
 target-microblaze/op_helper.c |8 +-
 target-mips/op_helper.c   |8 +-
 target-openrisc/mmu_helper.c  |   10 +-
 target-ppc/mem_helper.c   |8 +-
 target-s390x/mem_helper.c |8 +-
 target-sh4/op_helper.c|   23 +-
 target-sparc/cpu.h|1 -
 target-sparc/helper.c |   12 +-
 target-sparc/ldst_helper.c|   24 +-
 target-unicore32/op_helper.c  |9 +-
 target-xtensa/op_helper.c |   14 +-
 translate-all.c   | 1734 -
 translate-all.h   |   34 +
 user-exec.c   |8 +-
 25 files changed, 1814 insertions(+), 1829 deletions(-)
 create mode 100644 translate-all.h

-- 
1.7.2.5




Re: [Qemu-devel] [RFC] 1.4 release schedule

2012-12-04 Thread Anthony Liguori
Peter Maydell  writes:

> On 4 December 2012 18:38, Blue Swirl  wrote:
>> The definition of the hard freeze bothers me. A few patches that went
>> in after 1.3-rc0 were not bug fixes but just new features, so the
>> difference between soft and hard freezes was not clear.
>
> My vote for this would be to adhere to our definition
> and only commit bugfixes.

Let's get specific.  What was committed post hard freeze that's not a
bug fix?

The only thing I'm aware of is q35.  I asked Michael not to merge that
(he planned to) prior to the hard freeze because there was a specific
change I wanted to be made.  Normally, PCI bits would go through
Michael's tree but I felt the change really needed to be made.

So I agreed to take the bits post hard freeze so the change could be
made.  This is really was an exceptional case though and I don't think
it warrants a change in the description of hard freeze.  This was all
discussed on the mailing list too FWIW.

Regards,

Anthony Liguori

>
> -- PMM




Re: [Qemu-devel] [PATCH] configure: allow disabling pixman if not needed

2012-12-04 Thread Andreas Färber
Am 04.12.2012 16:58, schrieb Robert Schiele:
> When we build neither any system emulation targets nor the tools there
> is actually no need for pixman library.  In that case do not enforce
> presence of that library on the system.
> 
> Signed-off-by: Robert Schiele 

Reviewed-by: Andreas Färber 

Thanks for catching this. We usually build from the same .spec in two
passes so don't notice. There's probably more unnecessary dependencies
that could get thrown out for *-user if you so wanted.

Cheers,
Andreas

> ---
> This allows to reduce dependencies in case you build only user
> emulation targets.
> 
>  configure |   18 --
>  target-unicore32/helper.c |2 ++
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 994f731..7043b5a 100755
> --- a/configure
> +++ b/configure
> @@ -647,6 +647,8 @@ for opt do
>;;
>--without-system-pixman) pixman="internal"
>;;
> +  --without-pixman) pixman="none"
> +  ;;
>--disable-sdl) sdl="no"
>;;
>--enable-sdl) sdl="yes"
> @@ -2118,13 +2120,25 @@ fi
>  # pixman support probe
>  
>  if test "$pixman" = ""; then
> -  if $pkg_config pixman-1 > /dev/null 2>&1; then
> +  if test "$want_tools" = "no" -a "$softmmu" = "no"; then
> +pixman="none"
> +  elif $pkg_config pixman-1 > /dev/null 2>&1; then
>  pixman="system"
>else
>  pixman="internal"
>fi
>  fi
> -if test "$pixman" = "system"; then
> +if test "$pixman" = "none"; then
> +  if test "$want_tools" != "no" -o "$softmmu" != "no"; then
> +echo "ERROR: pixman disabled but system emulation or tools build"
> +echo "   enabled.  You can turn off pixman only if you also"
> +echo "   disable all system emulation targets and the tools"
> +echo "   build with '--disable-tools --disable-system'."
> +exit 1
> +  fi
> +  pixman_cflags=
> +  pixman_libs=
> +elif test "$pixman" = "system"; then
>pixman_cflags=`$pkg_config --cflags pixman-1 2>/dev/null`
>pixman_libs=`$pkg_config --libs pixman-1 2>/dev/null`
>  else
> diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
> index a9e226b..3e0df33 100644
> --- a/target-unicore32/helper.c
> +++ b/target-unicore32/helper.c
> @@ -13,7 +13,9 @@
>  #include "gdbstub.h"
>  #include "helper.h"
>  #include "host-utils.h"
> +#ifndef CONFIG_USER_ONLY
>  #include "console.h"
> +#endif
>  
>  #undef DEBUG_UC32
>  
> 


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



Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Richard Henderson
On 2012-12-04 13:00, Blue Swirl wrote:
>> > +if (rs5_0 == 0)
>> > +return;
> The check should be moved to translation time so that the call to this
> helper is not generated at all.

No, we'd do that only if this value were an immediate.
Branch over helper is not an optimization for an edge case runtime value.


r~



Re: [Qemu-devel] [Qemu-ppc] [PATCH 08/13] pseries: Update SLOF for NVRAM support

2012-12-04 Thread David Gibson
On Tue, Dec 04, 2012 at 09:27:54PM -0200, Erlon Cruz wrote:
> Em 04/12/2012 20:58, "David Gibson"  escreveu:
> >
> > On Tue, Dec 04, 2012 at 11:20:03AM -0200, Erlon Cruz wrote:
> > > On Tue, Dec 4, 2012 at 12:42 AM, David Gibson
> > > wrote:
> > >
> > > > Now that we have implemented PAPR compatible NVRAM interfaces in
> qemu, this
> > > > updates the SLOF firmware to actually initialize and use the NVRAM as
> a
> > > > PAPR guest firmware is expected to do.
> > > >
> > > > This SLOF update also includes an ugly but useful workaround for a
> bug in
> > > > the SLES11 installer which caused it to fail under KVM.
> > > >
> > > >
> > > I had a problem when installing SLES, when the installer et at 23%, a
> get:
> > >
> > > Installation of package ./suse/ppc64/vim-base-7.2-8.15.2.ppc64.rpm
> failed.
> > > Subprocess failed. Error: RPM failed: error:
> > > %post(vim-base-7.2-8.15.2.ppc64.rpm)
> > >
> > > Is this the same problem?
> >
> > No, I don't think so.  The NVRAM failure, I believe happens at the end
> > when it's doing the yaboot setup.  Although, I haven't met it myself,
> > so I'm not certain.
> 
> Have you succeed installing any SLES on QEMU pSeries? This is the first
> system I'm trying to install in an lpar. The guests I have I've installed
> in a power machine and then use the roots.

I haven't personally tried installing SLES, only Fedora and Debian
(and not very recently for those).  Some others in IBM have tried
installing SLES - it used to fail because of this bug and another, I'm
not sure if they've had a chance to try again since this fix.

Failing suddenly in the post script for vim-base seems a really
unlikely problem to be caused by qemu/host bugs, though anything's
possible.  First thing I'd try is to triple-check your install media.

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



Re: [Qemu-devel] [PATCH 5/6] add visitor for parsing hz[KMG] input string

2012-12-04 Thread Andreas Färber
Am 04.12.2012 20:34, schrieb Eduardo Habkost:
> From: Igor Mammedov 
> 
> Signed-off-by: Igor Mammedov 
> Acked-by: Andreas Färber 

Mike, do we need to do anything here wrt deallocation visitors?
Or can you ack?

Thanks,
Andreas

> ---
>  qapi/qapi-visit-core.c  | 11 +++
>  qapi/qapi-visit-core.h  |  2 ++
>  qapi/string-input-visitor.c | 22 ++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 7a82b63..5c8705e 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -311,3 +311,14 @@ void input_type_enum(Visitor *v, int *obj, const char 
> *strings[],
>  g_free(enum_str);
>  *obj = value;
>  }
> +
> +void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error 
> **errp)
> +{
> +if (!error_is_set(errp)) {
> +if (v->type_freq) {
> +v->type_freq(v, obj, name, errp);
> +} else {
> +v->type_int(v, obj, name, errp);
> +}
> +}
> +}
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index 60aceda..e5e7dd7 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -62,6 +62,7 @@ struct Visitor
>  void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  /* visit_type_size() falls back to (*type_uint64)() if type_size is 
> unset */
>  void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error 
> **errp);
> +void (*type_freq)(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  };
>  
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -91,5 +92,6 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char 
> *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error 
> **errp);
> +void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  
>  #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 497eb9a..74fe395 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool 
> *present,
>  *present = true;
>  }
>  
> +static void parse_type_freq(Visitor *v, int64_t *obj, const char *name,
> +Error **errp)
> +{
> +StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +char *endp = (char *) siv->string;
> +long long val = 0;
> +
> +errno = 0;
> +if (siv->string) {
> +val = strtosz_suffix_unit(siv->string, &endp,
> + STRTOSZ_DEFSUFFIX_B, 1000);
> +}
> +if (!siv->string || val == -1 || *endp) {
> +error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +  "a value representable as a non-negative int64");
> +return;
> +}
> +
> +*obj = val;
> +}
> +
>  Visitor *string_input_get_visitor(StringInputVisitor *v)
>  {
>  return &v->visitor;
> @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char 
> *str)
>  v->visitor.type_str = parse_type_str;
>  v->visitor.type_number = parse_type_number;
>  v->visitor.start_optional = parse_start_optional;
> +v->visitor.type_freq = parse_type_freq;
>  
>  v->string = str;
>  return v;
> 


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



Re: [Qemu-devel] vmstate conversion for virtio?

2012-12-04 Thread Rusty Russell
Juan Quintela  writes:

> Rusty Russell  wrote:
>> Hi all,
>>
>> I want to rework the qemu virtio subsystem, but various
>> structures are currently blatted to disk in save/load.  So I looked at
>> altering that, only to discover that it needs conversion to vmstate, and
>> 2009 patches in patchwork which have never been applied.
>>
>> Has there been any progress on this?  A git tree I should be using?
>
> My trees are more than 1 year old, and unfinished (basic trouble is how
> to express the lists of pending requests for block and now serial).  I
> haven't yet looked at virtio-scsi.

That's the easy part though, and part of what I want to change.

All we need is the index of the request; the rest can be re-read from
the ring.  So create the array in pre_save, and restore on post_load.

Is there something more recent than
http://repo.or.cz/w/qemu/quintela.git/shortlog/refs/heads/vmstate/virtio
or should I cherry-pick from there?

Thanks,
Rusty.



Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Jovanovic, Petar
> From: Andreas Färber [afaer...@suse.de]
>FWIW you could use our unlikely() macro then to aid branch prediction.

Just did. Thanks.

Petar


[Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Petar Jovanovic
From: Petar Jovanovic 

helper_shilo has not been shifting an accumulator value correctly for negative
values in 'shift' field. Minor optimization for shift=0 case.
This change also adds tests that will trigger issue and check for regressions.

Signed-off-by: Petar Jovanovic 
---
 target-mips/dsp_helper.c   |   17 +
 tests/tcg/mips/mips32-dsp/shilo.c  |   18 ++
 tests/tcg/mips/mips32-dsp/shilov.c |   20 
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index e7949c2..44f6dc7 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, target_ulong rs, 
CPUMIPSState *env)
 
 rs5_0 = rs & 0x3F;
 rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
-rs5_0 = MIPSDSP_ABS(rs5_0);
+
+if (unlikely(rs5_0 == 0)) {
+return;
+}
+
 acc   = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) |
 ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO);
-if (rs5_0 == 0) {
-temp = acc;
+
+if (rs5_0 > 0) {
+temp = acc >> rs5_0;
 } else {
-if (rs5_0 > 0) {
-temp = acc >> rs5_0;
-} else {
-temp = acc << rs5_0;
-}
+temp = acc << -rs5_0;
 }
 
 env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & MIPSDSP_LHI) >> 
32);
diff --git a/tests/tcg/mips/mips32-dsp/shilo.c 
b/tests/tcg/mips/mips32-dsp/shilo.c
index b686616..ce8ebc6 100644
--- a/tests/tcg/mips/mips32-dsp/shilo.c
+++ b/tests/tcg/mips/mips32-dsp/shilo.c
@@ -23,5 +23,23 @@ int main()
 assert(ach == resulth);
 assert(acl == resultl);
 
+
+ach = 0x1;
+acl = 0x8000;
+
+resulth = 0x3;
+resultl = 0x0;
+
+__asm
+("mthi %0, $ac1\n\t"
+ "mtlo %1, $ac1\n\t"
+ "shilo $ac1, -1\n\t"
+ "mfhi %0, $ac1\n\t"
+ "mflo %1, $ac1\n\t"
+ : "+r"(ach), "+r"(acl)
+);
+assert(ach == resulth);
+assert(acl == resultl);
+
 return 0;
 }
diff --git a/tests/tcg/mips/mips32-dsp/shilov.c 
b/tests/tcg/mips/mips32-dsp/shilov.c
index f186032..e1d6cea 100644
--- a/tests/tcg/mips/mips32-dsp/shilov.c
+++ b/tests/tcg/mips/mips32-dsp/shilov.c
@@ -25,5 +25,25 @@ int main()
 assert(ach == resulth);
 assert(acl == resultl);
 
+
+rs  = 0x;
+ach = 0x1;
+acl = 0x8000;
+
+resulth = 0x3;
+resultl = 0x0;
+
+__asm
+("mthi %0, $ac1\n\t"
+ "mtlo %1, $ac1\n\t"
+ "shilov $ac1, %2\n\t"
+ "mfhi %0, $ac1\n\t"
+ "mflo %1, $ac1\n\t"
+ : "+r"(ach), "+r"(acl)
+ : "r"(rs)
+);
+assert(ach == resulth);
+assert(acl == resultl);
+
 return 0;
 }
-- 
1.7.5.4




Re: [Qemu-devel] [Qemu-ppc] [PATCH 08/13] pseries: Update SLOF for NVRAM support

2012-12-04 Thread Erlon Cruz
Em 04/12/2012 20:58, "David Gibson"  escreveu:
>
> On Tue, Dec 04, 2012 at 11:20:03AM -0200, Erlon Cruz wrote:
> > On Tue, Dec 4, 2012 at 12:42 AM, David Gibson
> > wrote:
> >
> > > Now that we have implemented PAPR compatible NVRAM interfaces in
qemu, this
> > > updates the SLOF firmware to actually initialize and use the NVRAM as
a
> > > PAPR guest firmware is expected to do.
> > >
> > > This SLOF update also includes an ugly but useful workaround for a
bug in
> > > the SLES11 installer which caused it to fail under KVM.
> > >
> > >
> > I had a problem when installing SLES, when the installer et at 23%, a
get:
> >
> > Installation of package ./suse/ppc64/vim-base-7.2-8.15.2.ppc64.rpm
failed.
> > Subprocess failed. Error: RPM failed: error:
> > %post(vim-base-7.2-8.15.2.ppc64.rpm)
> >
> > Is this the same problem?
>
> No, I don't think so.  The NVRAM failure, I believe happens at the end
> when it's doing the yaboot setup.  Although, I haven't met it myself,
> so I'm not certain.

Have you succeed installing any SLES on QEMU pSeries? This is the first
system I'm trying to install in an lpar. The guests I have I've installed
in a power machine and then use the roots.

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


[Qemu-devel] [PATCH 3/3] target-i386:slightly refactor dr7 related function

2012-12-04 Thread liguang
Signed-off-by: liguang 
---
 target-i386/helper.c  |   70 +
 target-i386/machine.c |2 +-
 target-i386/misc_helper.c |4 +-
 target-i386/seg_helper.c  |6 ++--
 4 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index bf206cf..28307a1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -966,30 +966,31 @@ hwaddr cpu_get_phys_page_debug(CPUX86State *env, 
target_ulong addr)
 
 void hw_breakpoint_insert(CPUX86State *env, int index)
 {
-int type, err = 0;
+int type = 0, err = 0;
 
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
-if (hw_breakpoint_enabled(env->dr[7], index))
+case DR7_BP_INST:
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 err = cpu_breakpoint_insert(env, env->dr[index], BP_CPU,
 &env->cpu_breakpoint[index]);
+}
 break;
-case 1:
+case DR7_DATA_WR:
 type = BP_CPU | BP_MEM_WRITE;
-goto insert_wp;
-case 2:
- /* No support for I/O watchpoints yet */
-break;
-case 3:
+case DR7_DATA_RW:
 type = BP_CPU | BP_MEM_ACCESS;
-insert_wp:
+   case DR7_IO_RW:
+ /* No support for I/O watchpoints yet */
+ break;
+}
+   if (type) {
 err = cpu_watchpoint_insert(env, env->dr[index],
 hw_breakpoint_len(env->dr[7], index),
 type, &env->cpu_watchpoint[index]);
-break;
 }
-if (err)
+if (err) {
 env->cpu_breakpoint[index] = NULL;
+}
 }
 
 void hw_breakpoint_remove(CPUX86State *env, int index)
@@ -997,15 +998,16 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 if (!env->cpu_breakpoint[index])
 return;
 switch (hw_breakpoint_type(env->dr[7], index)) {
-case 0:
-if (hw_breakpoint_enabled(env->dr[7], index))
+case DR7_BP_INST:
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[index]);
+}
 break;
-case 1:
-case 3:
+case DR7_DATA_RW:
+case DR7_DATA_WR:
 cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[index]);
 break;
-case 2:
+case DR7_IO_RW:
 /* No support for I/O watchpoints yet */
 break;
 }
@@ -1014,22 +1016,40 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 int check_hw_breakpoints(CPUX86State *env, int force_dr6_update)
 {
 target_ulong dr6;
-int reg, type;
+int index;
 int hit_enabled = 0;
+bool bp_match = false;
+bool wp_match = false;
 
 dr6 = env->dr[6] & ~0xf;
-for (reg = 0; reg < 4; reg++) {
-type = hw_breakpoint_type(env->dr[7], reg);
-if ((type == 0 && env->dr[reg] == env->eip) ||
-((type & 1) && env->cpu_watchpoint[reg] &&
- (env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT))) {
-dr6 |= 1 << reg;
-if (hw_breakpoint_enabled(env->dr[7], reg))
+   for (index = 0; index < DR7_MAX_BP; index++) {
+switch (hw_breakpoint_type(env->dr[7], index)) {
+case DR7_BP_INST:
+if (env->dr[index] == env->eip) {
+bp_match = true;
+}
+break;
+case DR7_DATA_WR:
+case DR7_DATA_RW:
+if (env->cpu_watchpoint[index] &&
+env->cpu_watchpoint[index]->flags & BP_WATCHPOINT_HIT) {
+wp_match = true;
+}
+case DR7_IO_RW:
+break;
+}
+if (bp_match || wp_match) {
+dr6 |= 1 << index;
+if (hw_breakpoint_enabled(env->dr[7], index)) {
 hit_enabled = 1;
+}
+bp_match = false;
+wp_match = false;
 }
 }
 if (hit_enabled || force_dr6_update)
 env->dr[6] = dr6;
+
 return hit_enabled;
 }
 
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 4771508..a4b1a1e 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -265,7 +265,7 @@ static int cpu_post_load(void *opaque, int version_id)
 
 cpu_breakpoint_remove_all(env, BP_CPU);
 cpu_watchpoint_remove_all(env, BP_CPU);
-for (i = 0; i < 4; i++)
+for (i = 0; i < DR7_MAX_BP; i++)
 hw_breakpoint_insert(env, i);
 
 tlb_flush(env, 1);
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index a020379..5ee0863 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -197,11 +197,11 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
target_ulong t0)
 env->dr[reg] = t0;
 hw_breakpoint_insert(env, reg);
 } else if (reg == 7) {
-for (i = 0; i < 4; i++) {
+for (i = 0; i < DR7_MAX_BP; i++) {
 hw_breakpoint_remove(env, i);
 }
 env->dr[7] = t0;
-for

Re: [Qemu-devel] [PATCH] target-arm: use type_register() instead of type_register_static()

2012-12-04 Thread Peter Maydell
On 28 November 2012 19:20, Eduardo Habkost  wrote:
> The type_register_static() interface is documented as:
>
>   type_register_static:
>   @info: The #TypeInfo of the new type.
>
>   @info and all of the strings it points to should exist for the life
>   time that the type is registered.
>
> But cpu_register() uses a stack variable for the 'info' argument, so it
> has to use type_register() instead of type_register_static().
>
> Signed-off-by: Eduardo Habkost 

Thanks, applied to target-arm.next.

-- PMM



Re: [Qemu-devel] [RFC 06/10] qdev: add stubs for vmstate register/unregister functions

2012-12-04 Thread Eduardo Habkost
On Mon, Dec 03, 2012 at 10:49:56PM +0100, Igor Mammedov wrote:
> On Fri, 30 Nov 2012 17:27:18 -0200
> Eduardo Habkost  wrote:
> 
> > Add vmstate stub functions, so that qdev.o can be used without savevm.o
> > when vmstate support is not necessary (i.e. by *-user).
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Originally submitted as:
> >   Subject: qdev-core: isolate vmstate handling into separate functions
> > 
> > Changes v1 -> v2:
> >  - Add GCC_WEAK_DECL to function declarations
> > 
> > Changes v2 -> v3:
> >  - Subject: qdev: add weak aliases for vmstate handling on qdev.c
> >  - Make vmstate_register_with_alias_id()/vmstate_unregister()
> >have GCC_WEAK versions, instead of creating a new function
> >  - Kept qdev_get_vmsd() inside qdev.c
> > 
> > Changss v3 -> v4:
> >  - Use the new QEMU_WEAK_ALIAS system instead of GCC_WEAK
> > 
> > Changes v4 -> v5:
> >  - Use the new libqemustub.a, instead of QEMU_WEAK_ALIAS
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  stubs/Makefile.objs |  1 +
> >  stubs/vmstate.c | 17 +
> >  2 files changed, 18 insertions(+)
> >  create mode 100644 stubs/vmstate.c
> > 
> > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> > index 035b29a..5557079 100644
> > --- a/stubs/Makefile.objs
> > +++ b/stubs/Makefile.objs
> > @@ -5,4 +5,5 @@ stub-obj-y += fdset-get-fd.o
> >  stub-obj-y += fdset-remove-fd.o
> >  stub-obj-y += get-fd.o
> >  stub-obj-y += set-fd-handler.o
> > +stub-obj-y += vmstate.o
> >  stub-obj-$(CONFIG_WIN32) += fd-register.o
> > diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> > new file mode 100644
> > index 000..bb17884
> > --- /dev/null
> > +++ b/stubs/vmstate.c
> > @@ -0,0 +1,17 @@
> > +#include "qemu-common.h"
> > +#include "vmstate.h"
> > +
> > +int vmstate_register_with_alias_id(DeviceState *dev,
> > +   int instance_id,
> > +   const VMStateDescription 
> > *vmsd,
> > +   void *base, int alias_id,
> > +   int required_for_version)
> > +{
> > +return 0;
> > +}
> > +
> > +void vmstate_unregister(DeviceState *dev,
> > +const VMStateDescription *vmsd,
> > +void *opaque)
> > +{
> > +}
> > -- 
> > 1.7.11.7
> > 
> commit 3bc2f570ec9f description says, there should be a 1 object
> file per stub /with a corresponding file name if 3bc2f570ec9f is used as a
> model/.

Well, the commit description says that "if you place each function in a
separate source file, object files for unused functions will not be
taken in", I don't see it as a requirement. In this case, pulling both
stubs is a feature: if a binary ends up using the vmstate_register stub,
we really want it to use the vmstate_unregister stub as well.

> 
> Paolo,
>  Is it ok to put several related stub functions in one file?
> 
> -- 
> Regards,
>   Igor
> 

-- 
Eduardo



Re: [Qemu-devel] [Qemu-ppc] [PATCH 08/13] pseries: Update SLOF for NVRAM support

2012-12-04 Thread David Gibson
On Tue, Dec 04, 2012 at 11:20:03AM -0200, Erlon Cruz wrote:
> On Tue, Dec 4, 2012 at 12:42 AM, David Gibson
> wrote:
> 
> > Now that we have implemented PAPR compatible NVRAM interfaces in qemu, this
> > updates the SLOF firmware to actually initialize and use the NVRAM as a
> > PAPR guest firmware is expected to do.
> >
> > This SLOF update also includes an ugly but useful workaround for a bug in
> > the SLES11 installer which caused it to fail under KVM.
> >
> >
> I had a problem when installing SLES, when the installer et at 23%, a get:
> 
> Installation of package ./suse/ppc64/vim-base-7.2-8.15.2.ppc64.rpm failed.
> Subprocess failed. Error: RPM failed: error:
> %post(vim-base-7.2-8.15.2.ppc64.rpm)
> 
> Is this the same problem?

No, I don't think so.  The NVRAM failure, I believe happens at the end
when it's doing the yaboot setup.  Although, I haven't met it myself,
so I'm not certain.

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



Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversion

2012-12-04 Thread Andreas Färber
Ping,

Am 26.11.2012 01:12, schrieb Andreas Färber:
> Hello Anthony and Paolo,
> 
> As announced at KVM Forum, I have been preparing a new approach to 
> incrementally
> get us Anthony's QOM realizefn concept. A previous attempt by Paolo and me had
> been turned down for making this available at Object-level and over questions
> whether BlockDriverState may need its own three-stage realization model.
> 
> So here's an all-new patchset doing it at DeviceState-level only, adapting the
> signature to void realize(DeviceState *, Error **).
> CPUState is on a good way to get derived from DeviceState, so in the future
> will benefit from this approach as well.
> 
> I've picked ISADevice as an example to showcase what semantic effects the
> switch to QOM realizefn has (in the hopes the number of devices would be 
> small):
> As requested by Anthony for QOM CPUState reset and as seen with virtual 
> methods
> in object-oriented programming languages, it becomes the derived method's
> responsibility to decide when and whether to call the parent class' method. In
> lack of real vtables this requires to save the parent's method in case we want
> to call it; classes and matching macros may need to be added for that.
> Another point to note is that we should carefully distinguish what goes into
> the qdev initfn / QOM realizefn and what can already go into a QOM initfn.
> 
> This series is rebased onto Julien's ioport cleanups (touching on ISA 
> devices).
> 
> It starts by preparing the "realized" property, wrapping qdev's initfn (04).
> This means setting realized to true will not yet affect its children, as seen
> in Paolo's previous patches. That can be implemented later when the realizefns
> have been reviewed not to create new devices that would mess with recursive
> child realization. (In the previous series we had recursive realization but
> on my request dropped the hook-up of qdev due to the aforementioned quirks.)
> 
> At that point there is a coexistence of QOM device realizefn and qdev initfn.
> For the first time now I have set out to actually eliminate some qdev initfns,
> that's what I chose ISA for. This consists of three parts, introducing
> realizefns for ISADevices (28) and recursively for PITs (31) and for PICs 
> (34).
> As seen for the PCI host bridge series, I've extracted general QOM cleanups
> from the main conversion patch to arrive at a clean QOM'ish state while
> hopefully keeping the main patches readable.
> 
> This series also highlights an interesting find: Beyond the to-be-solved
> CPUState, there is also a "realize" function for BusState (02), which is not
> derived from DeviceState. :-)
> With the device-centric approach taken here it would still be possible to add
> "realized" properties to other types using their own infrastructure (e.g., a
> hardcoded setter rather than a realizefn hook).
> 
> Posted as an RFC to encourage bikeshedding, in particular about the type names
> and macros introduced. Adding new header files to move them out of the source
> files for, e.g., vl.c is left for a follow-up, but for instance I was unsure
> about TYPE_ISA_FDC (should this be TYPE_ISA_FLOPPY_DRIVE_CONTROLLER as with
> PCI_HOST_BRIDGE rather than PHB?), and naming of type names and functions is
> highly inconsistent (e.g., isa_vga vs. vga_isa, or pic vs. i8259).

So far I've heard no comments on the main QOM/ISA part of the series...

Anthony, did I understand you correctly wrt the method overriding?
(PIC/PIT) Or did you have this differently in mind?

Anyone any suggestions for improvement? If not, I'll send out a v2 this
week, with qdev_free() either rebased onto Paolo or removal dropped.
qbus was not further touched in the series, so I'd probably defer the
qbus_realize() change.

Branch below is rebased to match v1.3.0 + memory-ioport pull request.

Regards,
Andreas

> 
> Available for viewing/testing at:
> https://github.com/afaerber/qemu-cpu/commits/realize-qdev
> git://github.com/afaerber/qemu-cpu.git realize-qdev
[...]
> Andreas Färber (34):
>   qdev: Eliminate qdev_free() in favor of QOM

(depends on what we want to replace qdev_free() with)

>   qbus: QOM'ify qbus_realize()

(unrelated since qbus did not get a "realized" property in this series)

>   qdev: Fold state enum into bool realized
>   qdev: Prepare "realized" property
>   isa: Split off instance_init for ISADevice
>   applesmc: QOM'ify
>   cirrus_vga: QOM'ify ISA Cirrus VGA
>   debugcon: QOM'ify ISA debug console
>   fdc: QOM'ify ISA floppy controller
>   i82374: QOM'ify

>   i8259: Fix PIC_COMMON() macro

(applied already)

>   i8259: QOM cleanups
>   ide: QOM'ify ISA IDE
>   m48t59: QOM'ify ISA M48T59 NVRAM
>   mc146818rtc: QOM'ify
>   ne2000-isa: QOM'ify
>   parallel: QOM'ify
>   pc: QOM'ify port 92
>   pckbd: QOM'ify
>   pcspk: QOM'ify
>   sb16: QOM'ify
>   serial: QOM'ify ISA serial
>   sga: QOM'ify
>   vga-isa: QOM'ify ISA VGA
>   vmmouse: QOM'ify
>   vmport: QOM'ify
>   wdt_ib700: QOM'ify
>   isa: Use realizefn for ISAD

Re: [Qemu-devel] [PATCH 1/1] tmp105: Create API for TMP105 temperature sensor.

2012-12-04 Thread Andreas Färber
Am 04.12.2012 20:23, schrieb Alex Horn:
> * Define constants for TMP105 registers.
> * Move tmp105_set() from I2C to TMP105 header.
> 
> Signed-off-by: Alex Horn 

CC'ing TMP105 author and ARM maintainer. Comments inline.

> ---
>  hw/i2c.h|3 ---
>  hw/tmp105.c |   17 +
>  hw/tmp105.h |   34 ++
>  3 files changed, 43 insertions(+), 11 deletions(-)
>  create mode 100644 hw/tmp105.h
> 
> diff --git a/hw/i2c.h b/hw/i2c.h
> index 0f5682b..883b5c5 100644
> --- a/hw/i2c.h
> +++ b/hw/i2c.h
> @@ -73,9 +73,6 @@ void *wm8750_dac_buffer(void *opaque, int samples);
>  void wm8750_dac_commit(void *opaque);
>  void wm8750_set_bclk_in(void *opaque, int new_hz);
>  
> -/* tmp105.c */
> -void tmp105_set(I2CSlave *i2c, int temp);
> -
>  /* lm832x.c */
>  void lm832x_key_event(DeviceState *dev, int key, int state);
>  
> diff --git a/hw/tmp105.c b/hw/tmp105.c
> index 8e8dbd9..9c67e64 100644
> --- a/hw/tmp105.c
> +++ b/hw/tmp105.c
> @@ -20,6 +20,7 @@
>  
>  #include "hw.h"
>  #include "i2c.h"
> +#include "tmp105.h"
>  
>  typedef struct {
>  I2CSlave i2c;
> @@ -92,22 +93,22 @@ static void tmp105_read(TMP105State *s)
>  }
>  
>  switch (s->pointer & 3) {
> -case 0:  /* Temperature */
> +case TMP105_REG_TEMPERATURE:
>  s->buf[s->len ++] = (((uint16_t) s->temperature) >> 8);
>  s->buf[s->len ++] = (((uint16_t) s->temperature) >> 0) &
>  (0xf0 << ((~s->config >> 5) & 3));   /* R */
>  break;
>  
> -case 1:  /* Configuration */
> +case TMP105_REG_CONFIG:
>  s->buf[s->len ++] = s->config;
>  break;
>  
> -case 2:  /* T_LOW */
> +case TMP105_REG_T_LOW:
>  s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 8;
>  s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 0;
>  break;
>  
> -case 3:  /* T_HIGH */
> +case TMP105_REG_T_HIGH:
>  s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 8;
>  s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
>  break;
> @@ -117,10 +118,10 @@ static void tmp105_read(TMP105State *s)
>  static void tmp105_write(TMP105State *s)
>  {
>  switch (s->pointer & 3) {
> -case 0:  /* Temperature */
> +case TMP105_REG_TEMPERATURE:
>  break;
>  
> -case 1:  /* Configuration */
> +case TMP105_REG_CONFIG:
>  if (s->buf[0] & ~s->config & (1 << 0))   /* SD */
>  printf("%s: TMP105 shutdown\n", __FUNCTION__);
>  s->config = s->buf[0];
> @@ -128,8 +129,8 @@ static void tmp105_write(TMP105State *s)
>  tmp105_alarm_update(s);
>  break;
>  
> -case 2:  /* T_LOW */
> -case 3:  /* T_HIGH */
> +case TMP105_REG_T_LOW:
> +case TMP105_REG_T_HIGH:
>  if (s->len >= 3)
>  s->limit[s->pointer & 1] = (int16_t)
>  uint16_t) s->buf[0]) << 8) | s->buf[1]);
> diff --git a/hw/tmp105.h b/hw/tmp105.h
> new file mode 100644
> index 000..52aa4c9
> --- /dev/null
> +++ b/hw/tmp105.h

Adding a dedicated header is a good idea. In the future we may want to
move TMP105State struct there, too, and add a TYPE_TMP105 constant.

However, a new header should get a description and appropriate license,
since you are moving a declaration from tmp105.c here that probably
means GPLv2+.

> @@ -0,0 +1,34 @@
> +#ifndef QEMU_TMP105_H
> +#define QEMU_TMP105_H
> +
> +#include "i2c.h"
> +
> +/* The following temperature sensors are
> + * compatible with the TMP105 registers:
> + *
> + *   adt75
> + *   ds1775
> + *   ds75
> + *   lm75
> + *   lm75a
> + *   max6625
> + *   max6626
> + *   mcp980x
> + *   stds75
> + *   tcn75
> + *   tmp100
> + *   tmp101
> + *   tmp105
> + *   tmp175
> + *   tmp275
> + *   tmp75
> + */
> +#define TMP105_REG_TEMPERATURE 0
> +#define TMP105_REG_CONFIG  1
> +#define TMP105_REG_T_LOW   2 /* also known as T_hyst (e.g. LM75) */
> +#define TMP105_REG_T_HIGH  3 /* also known as T_OS (e.g. LM75) */

Might it make sense to use an enum here to improve the gdb experience?

> +
> +/* See also I2C_SLAVE_FROM_QDEV macro */

This comment did not exist before and I veto it: Instead, people should
be using the new I2C_SLAVE() macro, which works for Object just as well
as for DeviceState.

(git-grep counts six I2C_SLAVE_FROM_QDEV() users only, but I'd rather
get my ISA series merged before I send patches for yet another bus.)

> +void tmp105_set(I2CSlave *i2c, int temp);

Could you add a proper gtk-doc-style comment for this function,
documentation its parameters? Like, is @temp in Celsius, Fahrenheit,
Kelvin? Any range limits?

> +
> +#endif

Regards,
Andreas

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



Re: [Qemu-devel] [PATCH 4/4] exec: refactor cpu_restore_state

2012-12-04 Thread Peter Maydell
On 4 December 2012 21:39, Richard Henderson  wrote:
> On 2012-12-04 15:25, Peter Maydell wrote:
>> So this is just a refactoring, but it prompts me to ask -- how does
>> this work if the PC that caused us to take this TLB fill is legitimately
>> zero? We seem to be overloading retaddr==0 as a "not a real cpu fault"
>> indicator...
>
> Since this is a host code address, usually inside code_gen_buffer,
> not a target code address, this isn't ever going to happen.

Oh, right. I was confused by the fact we're keeping it in a uintptr_t
rather than a void*.

-- PMM



Re: [Qemu-devel] [RFC 0/8] CPU DeviceState v9

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 04:59:38PM +0100, Andreas Färber wrote:
> Am 04.12.2012 14:19, schrieb Eduardo Habkost:
> > Changes on v9:
> >  - Instead of moving qemu_[un]register_reset() to reset.c and including
> >it on *-user, create stubs for them on libqemustub.a
> 
> We compile cpu.c twice. Can't we do the same for qdev.c or whatever uses
> those functions? I feel they have no business being used in *-user.
> CC'ing Riku and Peter.

I don't understand what exactly you are suggesting. You suggest adding
#ifdefs to qdev.c to compile out the qemu_[un]register_reset() calls?

I had a version of this series that did exactly that[1], but IIRC
somebody suggested using stub functions instead. And I agree with
whoever suggested it, I believe stub functions are cleaner when the the
stub version still have the semantics expected by the caller[2].

[1] http://article.gmane.org/gmane.comp.emulators.xen.devel/137686
[2] e.g. a no-op qemu_register_reset() still does the job it's supposed
to do (making sure a function to be called when qemu_devices_reset()
is called), if we know qemu_devices_reset() is never called.


> 
> Andreas
> 
> >  - This is based on afaerber's qom-cpu branch, that has some header cleanup
> >changes. You can get the complete series in a git tree at:
> >https://github.com/ehabkost/qemu-hacks/tree/cpu_qdev.v9
> >git://github.com/ehabkost/qemu-hacks.git cpu_qdev.v9
> > 
> > v8:
> >  - Use a simpler copyright header on qdev-properties-system.c
> >  - Use the new libqemustub.a mechanism instead of the (now exting)
> >QEMU_WEAK_ALIAS mechanism
> >  - Move the reset-handler registration code to a new hw/reset.c file
> > 
> > v7:
> >  - Use the new QEMU_WEAK_ALIAS mechanism instead of the (now extinct)
> >GCC_WEAK attribute (patches 20 and 21)
> > 
> > v6:
> >  - Simple rebase against latest qemu.git master
> >  - Patch 13: some new typedefs were added and others were removed
> >  - Patch 19: trivial rebase
> > v5:
> >  - Tons of header cleanups just to eliminate qlist.h <-> cpu-common.h 
> > circular
> >dependency (patches 1-17)
> >  - Add copyright/license information to qdev-properties.c (patch 17)
> >  - Add copyright/license information to qdev-properties-system.c (patch 22)
> >  - use error_report()+abort() instead of hw_error() on qdev.c (patch 18)
> >  - Move qemu_[un]register_reset() and qemu_devices_reset() to qdev-core.c
> >(patch 19)
> >  - Make vmstate_[un]register() weak stubs, instead of a new function (patch 
> > 20)
> >  - Make sysbus_get_default() weak stub, instead of new qbus reset 
> > (un)register
> >functions (patch 21)
> >  - Eliminate qdev-system.c (all code is kept on qdev.c, now) (patch 22)
> > v4:
> >   - Add GCC_WEAK_DECL to functions that have GCC_WEAK versions
> >   - Updated the qdev_init_gpio_in() code on qdev-system.c to current version
> >   - Patch description updates (moved changelog below "---" and/or move info
> > about changes made by different authors between SoB lines)
> > v3 (submitted by Igor):
> >   - rebased on top of 8b4a3df (today's master)
> >   - slight code reshuffling in (see commit's changelog)
> >  "qdev: separate core from the code used only by qemu-system-*"
> >  "move qemu_irq typedef out of cpu-common.h"
> >   - commit messages cleanup
> > v2:
> >   Removes the CONFIG_USER_ONLY ifdefs, and use weak symbols to move
> >   the vmstate and qemu_register_reset() handling to qdev-system.c
> > 
> > git tree for testing:
> >   https://github.com/ehabkost/qemu-hacks/tree/cpu_qdev.v9
> >   git://github.com/ehabkost/qemu-hacks.git cpu_qdev.v9
> > 
> > References to previous versions:
> >   v8: http://article.gmane.org/gmane.comp.emulators.qemu/182589
> >   v7: http://article.gmane.org/gmane.comp.emulators.qemu/179969
> >   v6: http://article.gmane.org/gmane.comp.emulators.qemu/179918
> >   v5: http://article.gmane.org/gmane.comp.emulators.qemu/177426
> >   v4: http://article.gmane.org/gmane.comp.emulators.qemu/176127
> >   v3: http://article.gmane.org/gmane.comp.emulators.qemu/175980
> >   v2: http://article.gmane.org/gmane.comp.emulators.qemu/173909
> >   v1: http://article.gmane.org/gmane.comp.emulators.qemu/166630
> > 
> > 
> > Eduardo Habkost (7):
> >   move -I$(SRC_PATH)/include compiler flag to Makefile.objs
> >   qdev: qdev_create(): use error_report() instead of hw_error()
> >   libqemustub: add qemu_[un]register_reset() stubs
> >   libqemustub: vmstate register/unregister stubs
> >   libqemustub: sysbus_get_default() stub
> >   qdev-properties.c: separate core from the code used only by
> > qemu-system-*
> >   include qdev code into *-user, too
> > 
> > Igor Mammedov (1):
> >   qom: make CPU a child of DeviceState
> > 
> >  Makefile|   1 -
> >  Makefile.objs   |  23 ++-
> >  hw/Makefile.objs|  10 +-
> >  hw/qdev-properties-system.c | 352 
> > 
> >  hw/qdev-properties.c| 321 +---

Re: [Qemu-devel] detecting seccomp sandbox capability via QMP

2012-12-04 Thread Anthony Liguori
"Daniel P. Berrange"  writes:

> On Tue, Dec 04, 2012 at 01:13:46PM -0600, Anthony Liguori wrote:
>> "Daniel P. Berrange"  writes:
>> 
>> > On Tue, Dec 04, 2012 at 03:42:32PM +0100, Ján Tomko wrote:
>> >> On 12/04/12 12:46, Luiz Capitulino wrote:
>> >> > On Mon, 03 Dec 2012 16:55:35 +0100
>> >> > Ján Tomko  wrote:
>> >> > 
>> >> >> Hello,
>> >> >>
>> >> >> is there a way to check if QEMU was compiled with --enable-seccomp via 
>> >> >> QMP?
>> >> > 
>> >> > Not that I'm aware of. Could you describe your use-case?
>> >> 
>> >> It's for libvirt. The detection is broken since the switch from parsing
>> >> -help output to QMP and I wanted to fix it.
>> >> 
>> >> Assuming it's supported if we do capabilities detection via QMP (since
>> >> libvirt 1.0.0 and QEMU 1.2) would work except for this case:
>> >> If seccomp sandbox was requested in /etc/libvirt/qemu.conf, but it was
>> >> compiled out from qemu, libvirt would try to run QEMU with -sandbox on
>> >> instead of printing an error earlier.
>> >
>> > In the absence of any way to detect it via QMP, libvirt should fallback
>> > to hardcoding it based on the version number. This presumes that QEMU was
>> > built with it enabled in configure, but we've no other option for current
>> > released 1.2/1.3 versions.
>> 
>> echo quit | qemu -machine none -S -monitor stdio -vnc none -sandbox on
>> 
>> A non-zero execute means QEMU doesn't support the option.  This will
>> work for any new command line option introduction and can be considered
>> a "supported" way of probing for whether options are supported.
>
> One of the significant benefits to libvirt of the QMP based feature
> detection, was that we no longer have to invoke QEMU multiple times
> to query different data. I don't want to regress in this regard,
> because invoking QEMU many times has a noticable performance impact
> for some applications eg virt-sandbox were even 100ms delays are
> relevant.  So while what you describe does work, I don't think it
> is a satisfactory approach for libvirt.

Okay, so in terms of what exists today, I don't have a better option.
But we could add:

{ 'enum': 'ConfigEntryType',
  'data': [ 'number', 'string', 'bool', 'size' ] }

{ 'type': 'ConfigEntry',
  'data': { 'name': 'str', 'type': 'ConfigEntryType' } }

{ 'type': 'ConfigSection',
  'data': { 'name': 'str', 'fields': [ 'ConfigEntry' ] } }

{ 'command': 'query-config-schema',
  'returns': [ 'ConfigSection' ] }

This technically introspects config sections but obviously could be used
to detect the availability of -sandbox.

If it's useful, I can take a quick swing at implementing (or someone
else certainly could).

Regards,

Anthony Liguori

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



Re: [Qemu-devel] [PATCH 4/4] exec: refactor cpu_restore_state

2012-12-04 Thread Richard Henderson
On 2012-12-04 15:25, Peter Maydell wrote:
> So this is just a refactoring, but it prompts me to ask -- how does
> this work if the PC that caused us to take this TLB fill is legitimately
> zero? We seem to be overloading retaddr==0 as a "not a real cpu fault"
> indicator...

Since this is a host code address, usually inside code_gen_buffer,
not a target code address, this isn't ever going to happen.


r~



Re: [Qemu-devel] [PATCH 4/4] exec: refactor cpu_restore_state

2012-12-04 Thread Peter Maydell
On 4 December 2012 21:20, Blue Swirl  wrote:
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 6e3ab90..1fcc975 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -74,19 +74,13 @@ uint32_t HELPER(neon_tbl)(CPUARMState *env, uint32_t 
> ireg, uint32_t def,
>  void tlb_fill(CPUARMState *env, target_ulong addr, int is_write, int mmu_idx,
>uintptr_t retaddr)
>  {
> -TranslationBlock *tb;
>  int ret;
>
>  ret = cpu_arm_handle_mmu_fault(env, addr, is_write, mmu_idx);
>  if (unlikely(ret)) {
>  if (retaddr) {
>  /* now we have a real cpu fault */
> -tb = tb_find_pc(retaddr);
> -if (tb) {
> -/* the PC is inside the translated code. It means that we 
> have
> -   a virtual CPU fault */
> -cpu_restore_state(tb, env, retaddr);
> -}
> +cpu_restore_state(env, retaddr);
>  }
>  raise_exception(env, env->exception_index);
>  }

So this is just a refactoring, but it prompts me to ask -- how does
this work if the PC that caused us to take this TLB fill is legitimately
zero? We seem to be overloading retaddr==0 as a "not a real cpu fault"
indicator...

-- PMM



[Qemu-devel] [PATCH 2/4] exec: extract TB watchpoint check

2012-12-04 Thread Blue Swirl
Will be moved by the next patch.

Signed-off-by: Blue Swirl 
---
 exec.c |   22 ++
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index 6efd93e..7ee0650 100644
--- a/exec.c
+++ b/exec.c
@@ -2985,12 +2985,24 @@ static const MemoryRegionOps notdirty_mem_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void tb_check_watchpoint(CPUArchState *env)
+{
+TranslationBlock *tb;
+
+tb = tb_find_pc(env->mem_io_pc);
+if (!tb) {
+cpu_abort(env, "check_watchpoint: could not find TB for pc=%p",
+  (void *)env->mem_io_pc);
+}
+cpu_restore_state(tb, env, env->mem_io_pc);
+tb_phys_invalidate(tb, -1);
+}
+
 /* Generate a debug exception if a watchpoint has been hit.  */
 static void check_watchpoint(int offset, int len_mask, int flags)
 {
 CPUArchState *env = cpu_single_env;
 target_ulong pc, cs_base;
-TranslationBlock *tb;
 target_ulong vaddr;
 CPUWatchpoint *wp;
 int cpu_flags;
@@ -3009,13 +3021,7 @@ static void check_watchpoint(int offset, int len_mask, 
int flags)
 wp->flags |= BP_WATCHPOINT_HIT;
 if (!env->watchpoint_hit) {
 env->watchpoint_hit = wp;
-tb = tb_find_pc(env->mem_io_pc);
-if (!tb) {
-cpu_abort(env, "check_watchpoint: could not find TB for "
-  "pc=%p", (void *)env->mem_io_pc);
-}
-cpu_restore_state(tb, env, env->mem_io_pc);
-tb_phys_invalidate(tb, -1);
+tb_check_watchpoint(env);
 if (wp->flags & BP_STOP_BEFORE_ACCESS) {
 env->exception_index = EXCP_DEBUG;
 cpu_loop_exit(env);
-- 
1.7.2.5




Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Andreas Färber
Am 04.12.2012 20:48, schrieb Jovanovic, Petar:
>> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
>> index e7949c2..f8a7a9f 100644
>> --- a/target-mips/dsp_helper.c
>> +++ b/target-mips/dsp_helper.c
>> @@ -3814,17 +3814,17 @@ void helper_shilo(target_ulong ac, target_ulong rs, 
>> CPUMIPSState *env)
>>
>>  rs5_0 = rs & 0x3F;
>>  rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
>> -rs5_0 = MIPSDSP_ABS(rs5_0);
>> +
>> +if (rs5_0 == 0)
>> +return;
> 
>> The check should be moved to translation time so that the call to this
>> helper is not generated at all.
> 
> This case is not likely so generation of unnecessary call is unlikely too.
> Let me know what you think.

FWIW you could use our unlikely() macro then to aid branch prediction.

Andreas

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



Re: [Qemu-devel] [PATCH 1/4] hw/ds1338.c: Fix handling of HOURS register.

2012-12-04 Thread Peter Maydell
On 4 December 2012 20:57, Antoine Mathys  wrote:
> On 12/04/2012 06:42 PM, Peter Maydell wrote:
>> This looks good as far as the logic goes, but I think we could use some
>> symbolic constants for the 12-hour and PM bits rather than all the literal
>> 0x20 0x40 0x60. thanks -- PMM
>
> I refrained from using symbolic constants for three reasons:
> 1. You need to refer to the datasheet anyway to understand what is going on.
> 2. The code is short and it is easy to keep track of a few constants.
> 3. Symbolic names, which can be quite long, tend to make the code harder
> rather than easier to read

...I had the datasheet open and I still had to bounce back and forth
several times to work out which bits you were working with, so I
think we definitely disagree on this point...

> It is largely a matter of preference though and I'm not opposed to the idea.
> I would appreciate naming suggestions though.
>
> I propose not to have symbolic constants for the registers and to use CH,
> OSF, MODE12, PM for the various bits, provided such short names are
> acceptable. I'd really hate something like REG_HOURS_MODE12.

Your suggestions are definitely way too short; I'd go for something
in the middle like CTRL_OSF and HOURS_12HR, HOURS_PM.

-- PMM



Re: [Qemu-devel] [PATCH 1/4] hw/ds1338.c: Fix handling of HOURS register.

2012-12-04 Thread Antoine Mathys

On 12/04/2012 06:42 PM, Peter Maydell wrote:
This looks good as far as the logic goes, but I think we could use 
some symbolic constants for the 12-hour and PM bits rather than all 
the literal 0x20 0x40 0x60. thanks -- PMM 

I refrained from using symbolic constants for three reasons:
1. You need to refer to the datasheet anyway to understand what is going on.
2. The code is short and it is easy to keep track of a few constants.
3. Symbolic names, which can be quite long, tend to make the code harder 
rather than easier to read


It is largely a matter of preference though and I'm not opposed to the 
idea. I would appreciate naming suggestions though.


I propose not to have symbolic constants for the registers and to use 
CH, OSF, MODE12, PM for the various bits, provided such short names are 
acceptable. I'd really hate something like REG_HOURS_MODE12.





Re: [Qemu-devel] [PATCH 5/6] add visitor for parsing hz[KMG] input string

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 05:34:42PM -0200, Eduardo Habkost wrote:
> From: Igor Mammedov 
> 
> Signed-off-by: Igor Mammedov 
> Acked-by: Andreas Färber 

Reviewed-by: Eduardo Habkost 


> ---
>  qapi/qapi-visit-core.c  | 11 +++
>  qapi/qapi-visit-core.h  |  2 ++
>  qapi/string-input-visitor.c | 22 ++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 7a82b63..5c8705e 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -311,3 +311,14 @@ void input_type_enum(Visitor *v, int *obj, const char 
> *strings[],
>  g_free(enum_str);
>  *obj = value;
>  }
> +
> +void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error 
> **errp)
> +{
> +if (!error_is_set(errp)) {
> +if (v->type_freq) {
> +v->type_freq(v, obj, name, errp);
> +} else {
> +v->type_int(v, obj, name, errp);
> +}
> +}
> +}
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index 60aceda..e5e7dd7 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -62,6 +62,7 @@ struct Visitor
>  void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  /* visit_type_size() falls back to (*type_uint64)() if type_size is 
> unset */
>  void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error 
> **errp);
> +void (*type_freq)(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  };
>  
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -91,5 +92,6 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char 
> *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error 
> **errp);
> +void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error 
> **errp);
>  
>  #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 497eb9a..74fe395 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool 
> *present,
>  *present = true;
>  }
>  
> +static void parse_type_freq(Visitor *v, int64_t *obj, const char *name,
> +Error **errp)
> +{
> +StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +char *endp = (char *) siv->string;
> +long long val = 0;
> +
> +errno = 0;
> +if (siv->string) {
> +val = strtosz_suffix_unit(siv->string, &endp,
> + STRTOSZ_DEFSUFFIX_B, 1000);
> +}
> +if (!siv->string || val == -1 || *endp) {
> +error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +  "a value representable as a non-negative int64");
> +return;
> +}
> +
> +*obj = val;
> +}
> +
>  Visitor *string_input_get_visitor(StringInputVisitor *v)
>  {
>  return &v->visitor;
> @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char 
> *str)
>  v->visitor.type_str = parse_type_str;
>  v->visitor.type_number = parse_type_number;
>  v->visitor.start_optional = parse_start_optional;
> +v->visitor.type_freq = parse_type_freq;
>  
>  v->string = str;
>  return v;
> -- 
> 1.7.11.7
> 
> 

-- 
Eduardo



[Qemu-devel] [PATCH 6/6] target-i386: use visit_type_hz to parse tsc_freq property value

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Reviewed-by: Andreas Färber 
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 56a5646..5d11180 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1195,7 +1195,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 const int64_t max = INT64_MAX;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
+visit_type_freq(v, &value, name, errp);
 if (error_is_set(errp)) {
 return;
 }
-- 
1.7.11.7




[Qemu-devel] [PATCH 4/6] target-i386: postpone cpuid_level update to realize time

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

delay capping cpuid_level to 7 to realize time so property setters
for cpuid_7_0_ebx_features and "level" could be used in any order/time
between x86_cpu_initfn() and x86_cpu_realize().

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 05ac79a..56a5646 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1381,9 +1381,6 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
 goto error;
 }
-if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
-x86_cpu_def->level = 7;
-}
 return 0;
 
 error:
@@ -2074,6 +2071,11 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
 void x86_cpu_realize(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+
+if (env->cpuid_7_0_ebx_features && env->cpuid_level < 7) {
+env->cpuid_level = 7;
+}
 
 #ifndef CONFIG_USER_ONLY
 qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
-- 
1.7.11.7




Re: [Qemu-devel] Usage of Temperature Sensor (TMP105)

2012-12-04 Thread Alex Horn
Dear Andrzej,

> Most likely the function has never been in use.  It is there for
> completeness of the API.

I've submitted a patch to create a header file for the TMP105 API [1].
Having this API makes sense because the probe is compatible with
several others (as documented in the patch, see also [2]). In
addition, we would like to reuse the temperature hardware model in an
experiment. The new tmp105.h file would facilitate this.

With best regards,
Alex

[1] http://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg00363.html
[2] https://github.com/torvalds/linux/blob/master/drivers/hwmon/lm75.c#L37

On 3 December 2012 22:43, andrzej zaborowski  wrote:
> Hi Alex,
>
> On 1 December 2012 20:39, Alex Horn  wrote:
>> Hello all,
>>
>> As I have been browsing through QEMU's source code, I've noticed a
>> hardware model for a temperature sensor called TMP105. This model
>> implements the function tmp105_set(I2CSlave *i2c, int temp) declared
>> in i2c.h [0, 1].
>>
>> Surprisingly, however, I cannot find any code which calls this setter
>> function despite the fact that "CONFIG_TMP105=y" in at least one
>> target (e.g. [2]).
>>
>> I am keen to learn who uses this virtual device, i.e. who calls
>> tmp105_set(). This may answer my question if there exists a
>> "third-party" model which could simulate temperature changes in the
>> chip.
>
> Most likely the function has never been in use.  It is there for
> completeness of the API.  I was considering using it as a qemu monitor
> command if there is ever need to test the guest under changing
> temperature (e.g. some UI elements showing temperature changes).
>
> Cheers



Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Blue Swirl
On Tue, Dec 4, 2012 at 7:48 PM, Jovanovic, Petar  wrote:
>> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
>> index e7949c2..f8a7a9f 100644
>> --- a/target-mips/dsp_helper.c
>> +++ b/target-mips/dsp_helper.c
>> @@ -3814,17 +3814,17 @@ void helper_shilo(target_ulong ac, target_ulong rs, 
>> CPUMIPSState *env)
>>
>>  rs5_0 = rs & 0x3F;
>>  rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
>> -rs5_0 = MIPSDSP_ABS(rs5_0);
>> +
>> +if (rs5_0 == 0)
>> +return;
>
>> The check should be moved to translation time so that the call to this
>> helper is not generated at all.
>
> This case is not likely so generation of unnecessary call is unlikely too.
> Let me know what you think.

Sorry, I was confused.

> I will add the missing braces and I can also get rid of
> MIPSDSP_ABS(rs5_0).

OK.

>
> Petar



Re: [Qemu-devel] [PATCH 6/6] target-i386: use visit_type_hz to parse tsc_freq property value

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 05:34:43PM -0200, Eduardo Habkost wrote:
> From: Igor Mammedov 
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Andreas Färber 

Reviewed-by: Eduardo Habkost 

> ---
>  target-i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 56a5646..5d11180 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1195,7 +1195,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
> *v, void *opaque,
>  const int64_t max = INT64_MAX;
>  int64_t value;
>  
> -visit_type_int(v, &value, name, errp);
> +visit_type_freq(v, &value, name, errp);
>  if (error_is_set(errp)) {
>  return;
>  }
> -- 
> 1.7.11.7
> 
> 
> 
> 

-- 
Eduardo



Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Blue Swirl
On Tue, Dec 4, 2012 at 7:43 PM, Richard Henderson  wrote:
> On 2012-12-04 13:00, Blue Swirl wrote:
>>> > +if (rs5_0 == 0)
>>> > +return;
>> The check should be moved to translation time so that the call to this
>> helper is not generated at all.
>
> No, we'd do that only if this value were an immediate.
> Branch over helper is not an optimization for an edge case runtime value.

Right, for some reason I thought rs was a register number, sorry.

>
>
> r~



Re: [Qemu-devel] detecting seccomp sandbox capability via QMP

2012-12-04 Thread Daniel P. Berrange
On Tue, Dec 04, 2012 at 01:13:46PM -0600, Anthony Liguori wrote:
> "Daniel P. Berrange"  writes:
> 
> > On Tue, Dec 04, 2012 at 03:42:32PM +0100, Ján Tomko wrote:
> >> On 12/04/12 12:46, Luiz Capitulino wrote:
> >> > On Mon, 03 Dec 2012 16:55:35 +0100
> >> > Ján Tomko  wrote:
> >> > 
> >> >> Hello,
> >> >>
> >> >> is there a way to check if QEMU was compiled with --enable-seccomp via 
> >> >> QMP?
> >> > 
> >> > Not that I'm aware of. Could you describe your use-case?
> >> 
> >> It's for libvirt. The detection is broken since the switch from parsing
> >> -help output to QMP and I wanted to fix it.
> >> 
> >> Assuming it's supported if we do capabilities detection via QMP (since
> >> libvirt 1.0.0 and QEMU 1.2) would work except for this case:
> >> If seccomp sandbox was requested in /etc/libvirt/qemu.conf, but it was
> >> compiled out from qemu, libvirt would try to run QEMU with -sandbox on
> >> instead of printing an error earlier.
> >
> > In the absence of any way to detect it via QMP, libvirt should fallback
> > to hardcoding it based on the version number. This presumes that QEMU was
> > built with it enabled in configure, but we've no other option for current
> > released 1.2/1.3 versions.
> 
> echo quit | qemu -machine none -S -monitor stdio -vnc none -sandbox on
> 
> A non-zero execute means QEMU doesn't support the option.  This will
> work for any new command line option introduction and can be considered
> a "supported" way of probing for whether options are supported.

One of the significant benefits to libvirt of the QMP based feature
detection, was that we no longer have to invoke QEMU multiple times
to query different data. I don't want to regress in this regard,
because invoking QEMU many times has a noticable performance impact
for some applications eg virt-sandbox were even 100ms delays are
relevant.  So while what you describe does work, I don't think it
is a satisfactory approach for libvirt.

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



Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Jovanovic, Petar
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index e7949c2..f8a7a9f 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3814,17 +3814,17 @@ void helper_shilo(target_ulong ac, target_ulong rs, 
> CPUMIPSState *env)
>
>  rs5_0 = rs & 0x3F;
>  rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
> -rs5_0 = MIPSDSP_ABS(rs5_0);
> +
> +if (rs5_0 == 0)
> +return;

> The check should be moved to translation time so that the call to this
> helper is not generated at all.

This case is not likely so generation of unnecessary call is unlikely too.
Let me know what you think.

I will add the missing braces and I can also get rid of
MIPSDSP_ABS(rs5_0).

Petar


[Qemu-devel] [PATCH 3/6] target-i386: use define for cpuid vendor string size

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 6 +++---
 target-i386/cpu.h | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 70ba323..05ac79a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1106,13 +1106,13 @@ static char *x86_cpuid_get_vendor(Object *obj, Error 
**errp)
 char *value;
 int i;
 
-value = (char *)g_malloc(12 + 1);
+value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
 for (i = 0; i < 4; i++) {
 value[i] = env->cpuid_vendor1 >> (8 * i);
 value[i + 4] = env->cpuid_vendor2 >> (8 * i);
 value[i + 8] = env->cpuid_vendor3 >> (8 * i);
 }
-value[12] = '\0';
+value[CPUID_VENDOR_SZ] = '\0';
 return value;
 }
 
@@ -1123,7 +1123,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char 
*value,
 CPUX86State *env = &cpu->env;
 int i;
 
-if (strlen(value) != 12) {
+if (strlen(value) != CPUID_VENDOR_SZ) {
 error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
   "vendor", value);
 return;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 90ef1ff..386c4f6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -510,6 +510,8 @@
 #define CPUID_7_0_EBX_ADX  (1 << 19)
 #define CPUID_7_0_EBX_SMAP (1 << 20)
 
+#define CPUID_VENDOR_SZ  12
+
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
 #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
 #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH 2/6] target-i386: cpu: separate feature string parsing from CPU model lookup

2012-12-04 Thread Igor Mammedov
On Tue,  4 Dec 2012 17:34:39 -0200
Eduardo Habkost  wrote:

> Instead of using parsing the whole cpu_model string inside
> cpu_x86_find_by_name(), first split it into the CPU model name and the
> full feature string, then parse the feature string into pieces.
> 
> When using CPU model classes, those two pieces of information will be
> used at different moments (CPU model name will be used to find CPU
> class, feature string will be used after CPU object was created), so
> making the split in two steps will make it easier to refactor the code
> later.
> 
> This should also help on the CPU properties work, that will just need to
> replace the cpu_x86_parse_featurestr() logic (and can keep the CPU model
> lookup code as-is).
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
>  - Coding style changes
>  - Replace "goto error" with "return -1"
> 
> Changes v2 -> v3:
>  - Fix memory leak on handling of errors form object_property_set_str()
> ---
Reviewed-by: Igor Mammedov 

-- 
Regards,
  Igor



Re: [Qemu-devel] [PATCH 1/6] target-i386/cpu.c: coding style fixes

2012-12-04 Thread Igor Mammedov
On Tue,  4 Dec 2012 17:34:38 -0200
Eduardo Habkost  wrote:

> - Use spaces instead of tabs on cpu_x86_cpuid().
> - Use braces on 'if' statement cpu_x86_find_by_name().
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/cpu.c | 28 +++-
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index c6c2ca0..7afe839 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1227,9 +1227,11 @@ static int cpu_x86_find_by_name(x86_def_t 
> *x86_cpu_def, const char *cpu_model)
>  uint32_t minus_7_0_ebx_features = 0;
>  uint32_t numvalue;
>  
> -for (def = x86_defs; def; def = def->next)
> -if (name && !strcmp(name, def->name))
> +for (def = x86_defs; def; def = def->next) {
> +if (name && !strcmp(name, def->name)) {
>  break;
> +}
> +}
>  if (kvm_enabled() && name && strcmp(name, "host") == 0) {
>  kvm_cpu_fill_host(x86_cpu_def);
>  } else if (!def) {
> @@ -1835,17 +1837,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>  }
>  break;
>  case 0x800A:
> - if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
> - *eax = 0x0001; /* SVM Revision */
> - *ebx = 0x0010; /* nr of ASIDs */
> - *ecx = 0;
> - *edx = env->cpuid_svm_features; /* optional features */
> - } else {
> - *eax = 0;
> - *ebx = 0;
> - *ecx = 0;
> - *edx = 0;
> - }
> +if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
> +*eax = 0x0001; /* SVM Revision */
> +*ebx = 0x0010; /* nr of ASIDs */
> +*ecx = 0;
> +*edx = env->cpuid_svm_features; /* optional features */
> +} else {
> +*eax = 0;
> +*ebx = 0;
> +*ecx = 0;
> +*edx = 0;
> +}
>  break;
>  case 0xC000:
>  *eax = env->cpuid_xlevel2;
> -- 
> 1.7.11.7
> 
Reviewed-by: Igor Mammedov 

-- 
Regards,
  Igor



Re: [Qemu-devel] [PATCH 3/6] target-i386: use define for cpuid vendor string size

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 05:34:40PM -0200, Eduardo Habkost wrote:
> From: Igor Mammedov 
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Eduardo Habkost 

> ---
>  target-i386/cpu.c | 6 +++---
>  target-i386/cpu.h | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 70ba323..05ac79a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1106,13 +1106,13 @@ static char *x86_cpuid_get_vendor(Object *obj, Error 
> **errp)
>  char *value;
>  int i;
>  
> -value = (char *)g_malloc(12 + 1);
> +value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
>  for (i = 0; i < 4; i++) {
>  value[i] = env->cpuid_vendor1 >> (8 * i);
>  value[i + 4] = env->cpuid_vendor2 >> (8 * i);
>  value[i + 8] = env->cpuid_vendor3 >> (8 * i);
>  }
> -value[12] = '\0';
> +value[CPUID_VENDOR_SZ] = '\0';
>  return value;
>  }
>  
> @@ -1123,7 +1123,7 @@ static void x86_cpuid_set_vendor(Object *obj, const 
> char *value,
>  CPUX86State *env = &cpu->env;
>  int i;
>  
> -if (strlen(value) != 12) {
> +if (strlen(value) != CPUID_VENDOR_SZ) {
>  error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
>"vendor", value);
>  return;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 90ef1ff..386c4f6 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -510,6 +510,8 @@
>  #define CPUID_7_0_EBX_ADX  (1 << 19)
>  #define CPUID_7_0_EBX_SMAP (1 << 20)
>  
> +#define CPUID_VENDOR_SZ  12
> +
>  #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
>  #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
>  #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */
> -- 
> 1.7.11.7
> 
> 

-- 
Eduardo



Re: [Qemu-devel] [PATCH] target-i386: honor CR0_PG_MASK in cpu_get_phys_page_debug

2012-12-04 Thread Max Filippov
On Sun, Nov 18, 2012 at 12:52 AM, Max Filippov  wrote:
> cpu_get_phys_page_debug is not in sync with cpu_x86_handle_mmu_fault:
> the latter first checks CR0_PG_MASK and only after CR4_PAE_MASK.
>
> This fixes odd gdb code display with PAE enabled.
>
> Signed-off-by: Max Filippov 
> ---
>  target-i386/helper.c |   37 -
>  1 files changed, 20 insertions(+), 17 deletions(-)

Ping?

-- 
Thanks.
-- Max



[Qemu-devel] [PATCH 0/6] short x86 CPU init cleanup (v3)

2012-12-04 Thread Eduardo Habkost
Changes v3:
 - Fix memory leak spotted by Igor, on patch 2

Changes v2:
 - Coding style changes (patches 1-2)
 - Use "return -1" directly instead of "goto error" on cpu_x86_find_by_name()
   (patch 2)

This series is based on the qom-cpu branch from afaerber's tree. The full
branch can be found in a git tree, at:
  git://github.com/ehabkost/qemu-hacks.git cpu-init-cleanup-short-v3
  https://github.com/ehabkost/qemu-hacks/tree/cpu-init-cleanup-short-v3

References to previous versions:
 v2: http://marc.info/?l=qemu-devel&m=135464803607329
 v1: http://article.gmane.org/gmane.comp.emulators.qemu/182713
 previous (long) cleanup series:
   http://article.gmane.org/gmane.comp.emulators.qemu/180137


Eduardo Habkost (2):
  target-i386/cpu.c: coding style fixes
  target-i386: cpu: separate feature string parsing from CPU model
lookup

Igor Mammedov (4):
  target-i386: use define for cpuid vendor string size
  target-i386: postpone cpuid_level update to realize time
  add visitor for parsing hz[KMG] input string
  target-i386: use visit_type_hz to parse tsc_freq property value

 qapi/qapi-visit-core.c  |  11 +
 qapi/qapi-visit-core.h  |   2 +
 qapi/string-input-visitor.c |  22 ++
 target-i386/cpu.c   | 105 
 target-i386/cpu.h   |   2 +
 5 files changed, 103 insertions(+), 39 deletions(-)

-- 
1.7.11.7




Re: [Qemu-devel] [PATCH 4/6] target-i386: postpone cpuid_level update to realize time

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 05:34:41PM -0200, Eduardo Habkost wrote:
> From: Igor Mammedov 
> 
> delay capping cpuid_level to 7 to realize time so property setters
> for cpuid_7_0_ebx_features and "level" could be used in any order/time
> between x86_cpu_initfn() and x86_cpu_realize().
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Eduardo Habkost 


> ---
>  target-i386/cpu.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 05ac79a..56a5646 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1381,9 +1381,6 @@ static int cpu_x86_parse_featurestr(x86_def_t 
> *x86_cpu_def, char *features)
>  if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
>  goto error;
>  }
> -if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
> -x86_cpu_def->level = 7;
> -}
>  return 0;
>  
>  error:
> @@ -2074,6 +2071,11 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error 
> **errp)
>  void x86_cpu_realize(Object *obj, Error **errp)
>  {
>  X86CPU *cpu = X86_CPU(obj);
> +CPUX86State *env = &cpu->env;
> +
> +if (env->cpuid_7_0_ebx_features && env->cpuid_level < 7) {
> +env->cpuid_level = 7;
> +}
>  
>  #ifndef CONFIG_USER_ONLY
>  qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
> -- 
> 1.7.11.7
> 
> 

-- 
Eduardo



[Qemu-devel] [PATCH 2/6] target-i386: cpu: separate feature string parsing from CPU model lookup

2012-12-04 Thread Eduardo Habkost
Instead of using parsing the whole cpu_model string inside
cpu_x86_find_by_name(), first split it into the CPU model name and the
full feature string, then parse the feature string into pieces.

When using CPU model classes, those two pieces of information will be
used at different moments (CPU model name will be used to find CPU
class, feature string will be used after CPU object was created), so
making the split in two steps will make it easier to refactor the code
later.

This should also help on the CPU properties work, that will just need to
replace the cpu_x86_parse_featurestr() logic (and can keep the CPU model
lookup code as-is).

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 - Coding style changes
 - Replace "goto error" with "return -1"

Changes v2 -> v3:
 - Fix memory leak on handling of errors form object_property_set_str()
---
 target-i386/cpu.c | 69 ---
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7afe839..70ba323 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1208,25 +1208,10 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 cpu->env.tsc_khz = value / 1000;
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
+static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
-unsigned int i;
 x86_def_t *def;
 
-char *s = g_strdup(cpu_model);
-char *featurestr, *name = strtok(s, ",");
-/* Features to be added*/
-uint32_t plus_features = 0, plus_ext_features = 0;
-uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
-uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
-uint32_t plus_7_0_ebx_features = 0;
-/* Features to be removed */
-uint32_t minus_features = 0, minus_ext_features = 0;
-uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
-uint32_t minus_kvm_features = 0, minus_svm_features = 0;
-uint32_t minus_7_0_ebx_features = 0;
-uint32_t numvalue;
-
 for (def = x86_defs; def; def = def->next) {
 if (name && !strcmp(name, def->name)) {
 break;
@@ -1235,16 +1220,37 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 kvm_cpu_fill_host(x86_cpu_def);
 } else if (!def) {
-goto error;
+return -1;
 } else {
 memcpy(x86_cpu_def, def, sizeof(*def));
 }
 
+return 0;
+}
+
+/* Parse "+feature,-feature,feature=foo" CPU feature string
+ */
+static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features)
+{
+unsigned int i;
+char *featurestr; /* Single 'key=value" string being parsed */
+/* Features to be added*/
+uint32_t plus_features = 0, plus_ext_features = 0;
+uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
+uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
+uint32_t plus_7_0_ebx_features = 0;
+/* Features to be removed */
+uint32_t minus_features = 0, minus_ext_features = 0;
+uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
+uint32_t minus_kvm_features = 0, minus_svm_features = 0;
+uint32_t minus_7_0_ebx_features = 0;
+uint32_t numvalue;
+
 add_flagname_to_bitmaps("hypervisor", &plus_features,
 &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
 &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
 
-featurestr = strtok(NULL, ",");
+featurestr = features ? strtok(features, ",") : NULL;
 
 while (featurestr) {
 char *val;
@@ -1378,11 +1384,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
 x86_cpu_def->level = 7;
 }
-g_free(s);
 return 0;
 
 error:
-g_free(s);
 return -1;
 }
 
@@ -1492,11 +1496,25 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
 Error *error = NULL;
+char *name, *features;
+gchar **model_pieces;
 
 memset(def, 0, sizeof(*def));
 
-if (cpu_x86_find_by_name(def, cpu_model) < 0)
-return -1;
+model_pieces = g_strsplit(cpu_model, ",", 2);
+if (!model_pieces[0]) {
+goto error;
+}
+name = model_pieces[0];
+features = model_pieces[1];
+
+if (cpu_x86_find_by_name(def, name) < 0) {
+goto error;
+}
+
+if (cpu_x86_parse_featurestr(def, features) < 0) {
+goto error;
+}
 if (def->vendor1) {
 env->cpuid_vendor1 = def->vendor1;
 env->cpuid_vendor2 = def->vendor2;
@@ -1553,9 +1571,14 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 if (error) {
 fprintf(stderr, "%s\n", error_get_pretty(error));
 error_free(e

[Qemu-devel] [PATCH 5/6] add visitor for parsing hz[KMG] input string

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Acked-by: Andreas Färber 
---
 qapi/qapi-visit-core.c  | 11 +++
 qapi/qapi-visit-core.h  |  2 ++
 qapi/string-input-visitor.c | 22 ++
 3 files changed, 35 insertions(+)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..5c8705e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,14 @@ void input_type_enum(Visitor *v, int *obj, const char 
*strings[],
 g_free(enum_str);
 *obj = value;
 }
+
+void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+if (!error_is_set(errp)) {
+if (v->type_freq) {
+v->type_freq(v, obj, name, errp);
+} else {
+v->type_int(v, obj, name, errp);
+}
+}
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..e5e7dd7 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -62,6 +62,7 @@ struct Visitor
 void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
 /* visit_type_size() falls back to (*type_uint64)() if type_size is unset 
*/
 void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
+void (*type_freq)(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
 };
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -91,5 +92,6 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char 
*name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error 
**errp);
+void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 497eb9a..74fe395 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool *present,
 *present = true;
 }
 
+static void parse_type_freq(Visitor *v, int64_t *obj, const char *name,
+Error **errp)
+{
+StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+char *endp = (char *) siv->string;
+long long val = 0;
+
+errno = 0;
+if (siv->string) {
+val = strtosz_suffix_unit(siv->string, &endp,
+ STRTOSZ_DEFSUFFIX_B, 1000);
+}
+if (!siv->string || val == -1 || *endp) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+  "a value representable as a non-negative int64");
+return;
+}
+
+*obj = val;
+}
+
 Visitor *string_input_get_visitor(StringInputVisitor *v)
 {
 return &v->visitor;
@@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char 
*str)
 v->visitor.type_str = parse_type_str;
 v->visitor.type_number = parse_type_number;
 v->visitor.start_optional = parse_start_optional;
+v->visitor.type_freq = parse_type_freq;
 
 v->string = str;
 return v;
-- 
1.7.11.7




[Qemu-devel] [PATCH 1/6] target-i386/cpu.c: coding style fixes

2012-12-04 Thread Eduardo Habkost
- Use spaces instead of tabs on cpu_x86_cpuid().
- Use braces on 'if' statement cpu_x86_find_by_name().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c6c2ca0..7afe839 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1227,9 +1227,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 uint32_t minus_7_0_ebx_features = 0;
 uint32_t numvalue;
 
-for (def = x86_defs; def; def = def->next)
-if (name && !strcmp(name, def->name))
+for (def = x86_defs; def; def = def->next) {
+if (name && !strcmp(name, def->name)) {
 break;
+}
+}
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 kvm_cpu_fill_host(x86_cpu_def);
 } else if (!def) {
@@ -1835,17 +1837,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 }
 break;
 case 0x800A:
-   if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
-   *eax = 0x0001; /* SVM Revision */
-   *ebx = 0x0010; /* nr of ASIDs */
-   *ecx = 0;
-   *edx = env->cpuid_svm_features; /* optional features */
-   } else {
-   *eax = 0;
-   *ebx = 0;
-   *ecx = 0;
-   *edx = 0;
-   }
+if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+*eax = 0x0001; /* SVM Revision */
+*ebx = 0x0010; /* nr of ASIDs */
+*ecx = 0;
+*edx = env->cpuid_svm_features; /* optional features */
+} else {
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+}
 break;
 case 0xC000:
 *eax = env->cpuid_xlevel2;
-- 
1.7.11.7




[Qemu-devel] [PATCH 2/6] target-i386: cpu: separate feature string parsing from CPU model lookup

2012-12-04 Thread Eduardo Habkost
Instead of using parsing the whole cpu_model string inside
cpu_x86_find_by_name(), first split it into the CPU model name and the
full feature string, then parse the feature string into pieces.

When using CPU model classes, those two pieces of information will be
used at different moments (CPU model name will be used to find CPU
class, feature string will be used after CPU object was created), so
making the split in two steps will make it easier to refactor the code
later.

This should also help on the CPU properties work, that will just need to
replace the cpu_x86_parse_featurestr() logic (and can keep the CPU model
lookup code as-is).

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 - Coding style changes
 - Replace "goto error" with "return -1"
---
 target-i386/cpu.c | 67 +--
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7afe839..4090152 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1208,25 +1208,10 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 cpu->env.tsc_khz = value / 1000;
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
+static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
-unsigned int i;
 x86_def_t *def;
 
-char *s = g_strdup(cpu_model);
-char *featurestr, *name = strtok(s, ",");
-/* Features to be added*/
-uint32_t plus_features = 0, plus_ext_features = 0;
-uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
-uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
-uint32_t plus_7_0_ebx_features = 0;
-/* Features to be removed */
-uint32_t minus_features = 0, minus_ext_features = 0;
-uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
-uint32_t minus_kvm_features = 0, minus_svm_features = 0;
-uint32_t minus_7_0_ebx_features = 0;
-uint32_t numvalue;
-
 for (def = x86_defs; def; def = def->next) {
 if (name && !strcmp(name, def->name)) {
 break;
@@ -1235,16 +1220,37 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 kvm_cpu_fill_host(x86_cpu_def);
 } else if (!def) {
-goto error;
+return -1;
 } else {
 memcpy(x86_cpu_def, def, sizeof(*def));
 }
 
+return 0;
+}
+
+/* Parse "+feature,-feature,feature=foo" CPU feature string
+ */
+static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features)
+{
+unsigned int i;
+char *featurestr; /* Single 'key=value" string being parsed */
+/* Features to be added*/
+uint32_t plus_features = 0, plus_ext_features = 0;
+uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
+uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
+uint32_t plus_7_0_ebx_features = 0;
+/* Features to be removed */
+uint32_t minus_features = 0, minus_ext_features = 0;
+uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
+uint32_t minus_kvm_features = 0, minus_svm_features = 0;
+uint32_t minus_7_0_ebx_features = 0;
+uint32_t numvalue;
+
 add_flagname_to_bitmaps("hypervisor", &plus_features,
 &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
 &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
 
-featurestr = strtok(NULL, ",");
+featurestr = features ? strtok(features, ",") : NULL;
 
 while (featurestr) {
 char *val;
@@ -1378,11 +1384,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
 x86_cpu_def->level = 7;
 }
-g_free(s);
 return 0;
 
 error:
-g_free(s);
 return -1;
 }
 
@@ -1492,11 +1496,25 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
 Error *error = NULL;
+char *name, *features;
+gchar **model_pieces;
 
 memset(def, 0, sizeof(*def));
 
-if (cpu_x86_find_by_name(def, cpu_model) < 0)
-return -1;
+model_pieces = g_strsplit(cpu_model, ",", 2);
+if (!model_pieces[0]) {
+goto error;
+}
+name = model_pieces[0];
+features = model_pieces[1];
+
+if (cpu_x86_find_by_name(def, name) < 0) {
+goto error;
+}
+
+if (cpu_x86_parse_featurestr(def, features) < 0) {
+goto error;
+}
 if (def->vendor1) {
 env->cpuid_vendor1 = def->vendor1;
 env->cpuid_vendor2 = def->vendor2;
@@ -1555,7 +1573,12 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 error_free(error);
 return -1;
 }
+
+g_strfreev(model_pieces);
 return 0;
+error:
+g_strfreev(model_pieces);
+return -1;
 }
 
 #if !defined(CONFIG_USER_

[Qemu-devel] [PATCH 1/1] tmp105: Create API for TMP105 temperature sensor.

2012-12-04 Thread Alex Horn
* Define constants for TMP105 registers.
* Move tmp105_set() from I2C to TMP105 header.

Signed-off-by: Alex Horn 
---
 hw/i2c.h|3 ---
 hw/tmp105.c |   17 +
 hw/tmp105.h |   34 ++
 3 files changed, 43 insertions(+), 11 deletions(-)
 create mode 100644 hw/tmp105.h

diff --git a/hw/i2c.h b/hw/i2c.h
index 0f5682b..883b5c5 100644
--- a/hw/i2c.h
+++ b/hw/i2c.h
@@ -73,9 +73,6 @@ void *wm8750_dac_buffer(void *opaque, int samples);
 void wm8750_dac_commit(void *opaque);
 void wm8750_set_bclk_in(void *opaque, int new_hz);
 
-/* tmp105.c */
-void tmp105_set(I2CSlave *i2c, int temp);
-
 /* lm832x.c */
 void lm832x_key_event(DeviceState *dev, int key, int state);
 
diff --git a/hw/tmp105.c b/hw/tmp105.c
index 8e8dbd9..9c67e64 100644
--- a/hw/tmp105.c
+++ b/hw/tmp105.c
@@ -20,6 +20,7 @@
 
 #include "hw.h"
 #include "i2c.h"
+#include "tmp105.h"
 
 typedef struct {
 I2CSlave i2c;
@@ -92,22 +93,22 @@ static void tmp105_read(TMP105State *s)
 }
 
 switch (s->pointer & 3) {
-case 0:/* Temperature */
+case TMP105_REG_TEMPERATURE:
 s->buf[s->len ++] = (((uint16_t) s->temperature) >> 8);
 s->buf[s->len ++] = (((uint16_t) s->temperature) >> 0) &
 (0xf0 << ((~s->config >> 5) & 3)); /* R */
 break;
 
-case 1:/* Configuration */
+case TMP105_REG_CONFIG:
 s->buf[s->len ++] = s->config;
 break;
 
-case 2:/* T_LOW */
+case TMP105_REG_T_LOW:
 s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 8;
 s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 0;
 break;
 
-case 3:/* T_HIGH */
+case TMP105_REG_T_HIGH:
 s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 8;
 s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
 break;
@@ -117,10 +118,10 @@ static void tmp105_read(TMP105State *s)
 static void tmp105_write(TMP105State *s)
 {
 switch (s->pointer & 3) {
-case 0:/* Temperature */
+case TMP105_REG_TEMPERATURE:
 break;
 
-case 1:/* Configuration */
+case TMP105_REG_CONFIG:
 if (s->buf[0] & ~s->config & (1 << 0)) /* SD */
 printf("%s: TMP105 shutdown\n", __FUNCTION__);
 s->config = s->buf[0];
@@ -128,8 +129,8 @@ static void tmp105_write(TMP105State *s)
 tmp105_alarm_update(s);
 break;
 
-case 2:/* T_LOW */
-case 3:/* T_HIGH */
+case TMP105_REG_T_LOW:
+case TMP105_REG_T_HIGH:
 if (s->len >= 3)
 s->limit[s->pointer & 1] = (int16_t)
 uint16_t) s->buf[0]) << 8) | s->buf[1]);
diff --git a/hw/tmp105.h b/hw/tmp105.h
new file mode 100644
index 000..52aa4c9
--- /dev/null
+++ b/hw/tmp105.h
@@ -0,0 +1,34 @@
+#ifndef QEMU_TMP105_H
+#define QEMU_TMP105_H
+
+#include "i2c.h"
+
+/* The following temperature sensors are
+ * compatible with the TMP105 registers:
+ *
+ *   adt75
+ *   ds1775
+ *   ds75
+ *   lm75
+ *   lm75a
+ *   max6625
+ *   max6626
+ *   mcp980x
+ *   stds75
+ *   tcn75
+ *   tmp100
+ *   tmp101
+ *   tmp105
+ *   tmp175
+ *   tmp275
+ *   tmp75
+ */
+#define TMP105_REG_TEMPERATURE 0
+#define TMP105_REG_CONFIG  1
+#define TMP105_REG_T_LOW   2 /* also known as T_hyst (e.g. LM75) */
+#define TMP105_REG_T_HIGH  3 /* also known as T_OS (e.g. LM75) */
+
+/* See also I2C_SLAVE_FROM_QDEV macro */
+void tmp105_set(I2CSlave *i2c, int temp);
+
+#endif
-- 
1.7.6.5




[Qemu-devel] [PATCH 3/6] target-i386: use define for cpuid vendor string size

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 6 +++---
 target-i386/cpu.h | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4090152..86d7a61 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1106,13 +1106,13 @@ static char *x86_cpuid_get_vendor(Object *obj, Error 
**errp)
 char *value;
 int i;
 
-value = (char *)g_malloc(12 + 1);
+value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
 for (i = 0; i < 4; i++) {
 value[i] = env->cpuid_vendor1 >> (8 * i);
 value[i + 4] = env->cpuid_vendor2 >> (8 * i);
 value[i + 8] = env->cpuid_vendor3 >> (8 * i);
 }
-value[12] = '\0';
+value[CPUID_VENDOR_SZ] = '\0';
 return value;
 }
 
@@ -1123,7 +1123,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char 
*value,
 CPUX86State *env = &cpu->env;
 int i;
 
-if (strlen(value) != 12) {
+if (strlen(value) != CPUID_VENDOR_SZ) {
 error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
   "vendor", value);
 return;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 90ef1ff..386c4f6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -510,6 +510,8 @@
 #define CPUID_7_0_EBX_ADX  (1 << 19)
 #define CPUID_7_0_EBX_SMAP (1 << 20)
 
+#define CPUID_VENDOR_SZ  12
+
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
 #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
 #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */
-- 
1.7.11.7




[Qemu-devel] [PATCH 6/6] target-i386: use visit_type_hz to parse tsc_freq property value

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Reviewed-by: Andreas Färber 
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a56a130..f8ba569 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1195,7 +1195,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 const int64_t max = INT64_MAX;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
+visit_type_freq(v, &value, name, errp);
 if (error_is_set(errp)) {
 return;
 }
-- 
1.7.11.7




Re: [Qemu-devel] [RFC] 1.4 release schedule

2012-12-04 Thread Blue Swirl
On Tue, Dec 4, 2012 at 6:42 PM, Peter Maydell  wrote:
> On 4 December 2012 18:38, Blue Swirl  wrote:
>> The definition of the hard freeze bothers me. A few patches that went
>> in after 1.3-rc0 were not bug fixes but just new features, so the
>> difference between soft and hard freezes was not clear.
>
> My vote for this would be to adhere to our definition
> and only commit bugfixes.

With that approach, I'd change the durations a bit, something like 20
days for soft freeze and 10 for hard freeze (really hard, bug fixes
only with word 'fix' in commit message, no late pulls etc).

>
> -- PMM



Re: [Qemu-devel] detecting seccomp sandbox capability via QMP

2012-12-04 Thread Anthony Liguori
"Daniel P. Berrange"  writes:

> On Tue, Dec 04, 2012 at 03:42:32PM +0100, Ján Tomko wrote:
>> On 12/04/12 12:46, Luiz Capitulino wrote:
>> > On Mon, 03 Dec 2012 16:55:35 +0100
>> > Ján Tomko  wrote:
>> > 
>> >> Hello,
>> >>
>> >> is there a way to check if QEMU was compiled with --enable-seccomp via 
>> >> QMP?
>> > 
>> > Not that I'm aware of. Could you describe your use-case?
>> 
>> It's for libvirt. The detection is broken since the switch from parsing
>> -help output to QMP and I wanted to fix it.
>> 
>> Assuming it's supported if we do capabilities detection via QMP (since
>> libvirt 1.0.0 and QEMU 1.2) would work except for this case:
>> If seccomp sandbox was requested in /etc/libvirt/qemu.conf, but it was
>> compiled out from qemu, libvirt would try to run QEMU with -sandbox on
>> instead of printing an error earlier.
>
> In the absence of any way to detect it via QMP, libvirt should fallback
> to hardcoding it based on the version number. This presumes that QEMU was
> built with it enabled in configure, but we've no other option for current
> released 1.2/1.3 versions.

echo quit | qemu -machine none -S -monitor stdio -vnc none -sandbox on

A non-zero execute means QEMU doesn't support the option.  This will
work for any new command line option introduction and can be considered
a "supported" way of probing for whether options are supported.

Regards,

Anthony Liguori

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



Re: [Qemu-devel] [PATCH 2/6] target-i386: cpu: separate feature string parsing from CPU model lookup

2012-12-04 Thread Igor Mammedov
On Tue,  4 Dec 2012 16:58:19 -0200
Eduardo Habkost  wrote:

> Instead of using parsing the whole cpu_model string inside
> cpu_x86_find_by_name(), first split it into the CPU model name and the
> full feature string, then parse the feature string into pieces.
> 
> When using CPU model classes, those two pieces of information will be
> used at different moments (CPU model name will be used to find CPU
> class, feature string will be used after CPU object was created), so
> making the split in two steps will make it easier to refactor the code
> later.
> 
> This should also help on the CPU properties work, that will just need to
> replace the cpu_x86_parse_featurestr() logic (and can keep the CPU model
> lookup code as-is).
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
>  - Coding style changes
>  - Replace "goto error" with "return -1"
> ---
>  target-i386/cpu.c | 67 
> +--
>  1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 7afe839..4090152 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1208,25 +1208,10 @@ static void x86_cpuid_set_tsc_freq(Object *obj, 
> Visitor *v, void *opaque,
>  cpu->env.tsc_khz = value / 1000;
>  }
>  
> -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char 
> *cpu_model)
> +static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>  {
> -unsigned int i;
>  x86_def_t *def;
>  
> -char *s = g_strdup(cpu_model);
> -char *featurestr, *name = strtok(s, ",");
> -/* Features to be added*/
> -uint32_t plus_features = 0, plus_ext_features = 0;
> -uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
> -uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
> -uint32_t plus_7_0_ebx_features = 0;
> -/* Features to be removed */
> -uint32_t minus_features = 0, minus_ext_features = 0;
> -uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
> -uint32_t minus_kvm_features = 0, minus_svm_features = 0;
> -uint32_t minus_7_0_ebx_features = 0;
> -uint32_t numvalue;
> -
>  for (def = x86_defs; def; def = def->next) {
>  if (name && !strcmp(name, def->name)) {
>  break;
> @@ -1235,16 +1220,37 @@ static int cpu_x86_find_by_name(x86_def_t 
> *x86_cpu_def, const char *cpu_model)
>  if (kvm_enabled() && name && strcmp(name, "host") == 0) {
>  kvm_cpu_fill_host(x86_cpu_def);
>  } else if (!def) {
> -goto error;
> +return -1;
>  } else {
>  memcpy(x86_cpu_def, def, sizeof(*def));
>  }
>  
> +return 0;
> +}
> +
> +/* Parse "+feature,-feature,feature=foo" CPU feature string
> + */
> +static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features)
> +{
> +unsigned int i;
> +char *featurestr; /* Single 'key=value" string being parsed */
> +/* Features to be added*/
> +uint32_t plus_features = 0, plus_ext_features = 0;
> +uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
> +uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0;
> +uint32_t plus_7_0_ebx_features = 0;
> +/* Features to be removed */
> +uint32_t minus_features = 0, minus_ext_features = 0;
> +uint32_t minus_ext2_features = 0, minus_ext3_features = 0;
> +uint32_t minus_kvm_features = 0, minus_svm_features = 0;
> +uint32_t minus_7_0_ebx_features = 0;
> +uint32_t numvalue;
> +
>  add_flagname_to_bitmaps("hypervisor", &plus_features,
>  &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
>  &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
>  
> -featurestr = strtok(NULL, ",");
> +featurestr = features ? strtok(features, ",") : NULL;
>  
>  while (featurestr) {
>  char *val;
> @@ -1378,11 +1384,9 @@ static int cpu_x86_find_by_name(x86_def_t 
> *x86_cpu_def, const char *cpu_model)
>  if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
>  x86_cpu_def->level = 7;
>  }
> -g_free(s);
>  return 0;
>  
>  error:
> -g_free(s);
>  return -1;
>  }
>  
> @@ -1492,11 +1496,25 @@ int cpu_x86_register(X86CPU *cpu, const char 
> *cpu_model)
>  CPUX86State *env = &cpu->env;
>  x86_def_t def1, *def = &def1;
>  Error *error = NULL;
> +char *name, *features;
> +gchar **model_pieces;
>  
>  memset(def, 0, sizeof(*def));
>  
> -if (cpu_x86_find_by_name(def, cpu_model) < 0)
> -return -1;
> +model_pieces = g_strsplit(cpu_model, ",", 2);
> +if (!model_pieces[0]) {
> +goto error;
> +}
> +name = model_pieces[0];
> +features = model_pieces[1];
> +
> +if (cpu_x86_find_by_name(def, name) < 0) {
> +goto error;
> +}
> +
> +if (cpu_x86_parse_featurestr(def, features) < 0) {
> +goto error;
> +}
>  if (def->vendor1) {
>  env->cp

Re: [Qemu-devel] [RFC 6/8] qdev-properties.c: separate core from the code used only by qemu-system-*

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 08:05:43PM +0100, Andreas Färber wrote:
> Am 04.12.2012 20:01, schrieb Eduardo Habkost:
> > On Tue, Dec 04, 2012 at 06:55:17PM +, Blue Swirl wrote:
> >> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost  
> >> wrote:
> >>> +static int parse_drive(DeviceState *dev, const char *str, void **ptr)
> >>> +{
> >>> +BlockDriverState *bs;
> >>> +
> >>> +bs = bdrv_find(str);
> >>> +if (bs == NULL)
> >>
> >> Please add braces, use checkpatch.pl.
> >>
> >>> +return -ENOENT;
> >>> +if (bdrv_attach_dev(bs, dev) < 0)
> >>
> >> Also here.
> > 
> > 
> > This is pure code movement, and I won't introduce changes in the code
> > while it is being moved to not make patch review harder.
> 
> > If you want to send coding style patches for that code after it is
> > moved, be my guest.
> 
> No. Patches are required to pass checkpatch.pl (i.e., + lines in the
> patch need braces). That means if not in the same patch for movement
> reasons, Coding Style cleanups need to be done in advance, not as
> followup. (same issue as in his AREG0 series)

I had the feeling that the amount of cleanup (included but not limited
to coding style changes) I was doing before introducing actual code
changes was making some of my series harder to review. If making coding
style changes first is preferred, I can do it. I will try to send v10 of
this series today.

> 
> Andreas
> 
> P.S. Dropping unrelated parts of the quotes makes requested changes
> easier to spot. There was a ditto hidden somewhere down. ;)

ACK.  :)

-- 
Eduardo



Re: [Qemu-devel] [RFC 6/8] qdev-properties.c: separate core from the code used only by qemu-system-*

2012-12-04 Thread Andreas Färber
Am 04.12.2012 20:01, schrieb Eduardo Habkost:
> On Tue, Dec 04, 2012 at 06:55:17PM +, Blue Swirl wrote:
>> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost  wrote:
>>> +static int parse_drive(DeviceState *dev, const char *str, void **ptr)
>>> +{
>>> +BlockDriverState *bs;
>>> +
>>> +bs = bdrv_find(str);
>>> +if (bs == NULL)
>>
>> Please add braces, use checkpatch.pl.
>>
>>> +return -ENOENT;
>>> +if (bdrv_attach_dev(bs, dev) < 0)
>>
>> Also here.
> 
> 
> This is pure code movement, and I won't introduce changes in the code
> while it is being moved to not make patch review harder.

> If you want to send coding style patches for that code after it is
> moved, be my guest.

No. Patches are required to pass checkpatch.pl (i.e., + lines in the
patch need braces). That means if not in the same patch for movement
reasons, Coding Style cleanups need to be done in advance, not as
followup. (same issue as in his AREG0 series)

Andreas

P.S. Dropping unrelated parts of the quotes makes requested changes
easier to spot. There was a ditto hidden somewhere down. ;)

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



Re: [Qemu-devel] [RFC 6/8] qdev-properties.c: separate core from the code used only by qemu-system-*

2012-12-04 Thread Blue Swirl
On Tue, Dec 4, 2012 at 7:01 PM, Eduardo Habkost  wrote:
> On Tue, Dec 04, 2012 at 06:55:17PM +, Blue Swirl wrote:
>> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost  wrote:
>> > This separates the qdev properties code in two parts:
>> >  - qdev-properties.c, that contains most of the qdev properties code;
>> >  - qdev-properties-system.c for code specific for qemu-system-*,
>> >containing:
>> >- Property types: drive, chr, netdev, vlan, that depend on code that
>> >  won't be included on *-user
>> >- qemu_add_globals(), that depends on qemu-config.o.
>> >
>> > This change should help on two things:
>> >  - Allowing DeviceState to be used by *-user without pulling
>> >dependencies that are specific for qemu-system-*;
>> >  - Writing qdev unit tests without pulling too many dependencies.
>> >
>> > The copyright/license header for the new file is directly copied from
>> > qdev-properties.c.
>> >
>> > Signed-off-by: Eduardo Habkost 
>> > ---
>> > Detailed changelog:
>> >
>> > Changes v1 (ehabkost) -> v2 (imammedo):
>> >  - keep qdev_get_child_bus() in hw/qdev.c
>> >  - put qdev_set_nic_properties() in hw/qdev-properties-system.c
>> >
>> > Changes v2 -> v3 (ehabkost):
>> >  - updated the qdev_init_gpio_in() code on qdev-system.c to current
>> >version
>> >
>> > Changes v3 -> v4 (ehabkost):
>> >  - Added copyright/license information to qdev-properties-system.c
>> >(based on copyright/license of qdev-properties.c)
>> >  - Whitespace change at the end of qdev-properties.c
>> >  - Don't create qdev-system.c, now we can keep the qdev.c code as-is
>> >as the qdev.c dependencies were reduced
>> >  - Rewrite patch description
>> >
>> > Changes v4 -> v5 (ehabkost):
>> >  - Remove large copyright header and instead just point to the original
>> >file it was based on
>> >
>> > Changes v5 -> v6 (ehabkost):
>> >  - Removed inter-SoB line changelog from commit message
>> > ---
>> >  hw/Makefile.objs|   1 +
>> >  hw/qdev-properties-system.c | 352 
>> > 
>> >  hw/qdev-properties.c| 321 +---
>> >  hw/qdev-properties.h|   1 +
>> >  hw/qdev.c   |  13 --
>> >  5 files changed, 355 insertions(+), 333 deletions(-)
>> >  create mode 100644 hw/qdev-properties-system.c
>> >
>> > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
>> > index d581d8d..96a8365 100644
>> > --- a/hw/Makefile.objs
>> > +++ b/hw/Makefile.objs
>> > @@ -185,6 +185,7 @@ common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o 
>> > bt-hid.o
>> >  common-obj-y += bt-hci-csr.o
>> >  common-obj-y += msmouse.o ps2.o
>> >  common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
>> > +common-obj-y += qdev-properties-system.o
>> >  common-obj-$(CONFIG_BRLAPI) += baum.o
>> >
>> >  # xen backend driver support
>> > diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
>> > new file mode 100644
>> > index 000..9a7e0b3
>> > --- /dev/null
>> > +++ b/hw/qdev-properties-system.c
>> > @@ -0,0 +1,352 @@
>> > +/*
>> > + * qdev property parsing and global properties
>> > + * (parts specific for qemu-system-*)
>> > + *
>> > + * This file is based on code from hw/qdev-properties.c from
>> > + * commit 4e68f7a0819f179c2ff90a60611806c789911cc2,
>> > + * Copyright (c) Gerd Hoffmann  and other contributors.
>> > + *
>> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
>> > later.
>> > + * See the COPYING file in the top-level directory.
>> > + */
>> > +
>> > +#include "net.h"
>> > +#include "qdev.h"
>> > +#include "qerror.h"
>> > +#include "blockdev.h"
>> > +#include "hw/block-common.h"
>> > +#include "net/hub.h"
>> > +#include "qapi/qapi-visit-core.h"
>> > +
>> > +static void get_pointer(Object *obj, Visitor *v, Property *prop,
>> > +const char *(*print)(void *ptr),
>> > +const char *name, Error **errp)
>> > +{
>> > +DeviceState *dev = DEVICE(obj);
>> > +void **ptr = qdev_get_prop_ptr(dev, prop);
>> > +char *p;
>> > +
>> > +p = (char *) (*ptr ? print(*ptr) : "");
>> > +visit_type_str(v, &p, name, errp);
>> > +}
>> > +
>> > +static void set_pointer(Object *obj, Visitor *v, Property *prop,
>> > +int (*parse)(DeviceState *dev, const char *str,
>> > + void **ptr),
>> > +const char *name, Error **errp)
>> > +{
>> > +DeviceState *dev = DEVICE(obj);
>> > +Error *local_err = NULL;
>> > +void **ptr = qdev_get_prop_ptr(dev, prop);
>> > +char *str;
>> > +int ret;
>> > +
>> > +if (dev->state != DEV_STATE_CREATED) {
>> > +error_set(errp, QERR_PERMISSION_DENIED);
>> > +return;
>> > +}
>> > +
>> > +visit_type_str(v, &str, name, &local_err);
>> > +if (local_err) {
>> > +error_propagate(errp, local_err);
>> > +return;
>> > +}
>> > +if (!*str) {
>> > +g_free(str);
>> > +

Re: [Qemu-devel] [PATCH] target-mips: Fix incorrect shift for SHILO and SHILOV

2012-12-04 Thread Blue Swirl
On Tue, Dec 4, 2012 at 2:49 PM, Petar Jovanovic
 wrote:
> From: Petar Jovanovic 
>
> helper_shilo has not been shifting an accumulator value correctly for negative
> values in 'shift' field. Minor optimization for shift=0 case.
> This change also adds tests that will trigger issue and check for regressions.
>
> Signed-off-by: Petar Jovanovic 
> ---
>  target-mips/dsp_helper.c   |   16 
>  tests/tcg/mips/mips32-dsp/shilo.c  |   18 ++
>  tests/tcg/mips/mips32-dsp/shilov.c |   20 
>  3 files changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index e7949c2..f8a7a9f 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3814,17 +3814,17 @@ void helper_shilo(target_ulong ac, target_ulong rs, 
> CPUMIPSState *env)
>
>  rs5_0 = rs & 0x3F;
>  rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
> -rs5_0 = MIPSDSP_ABS(rs5_0);
> +
> +if (rs5_0 == 0)
> +return;

The check should be moved to translation time so that the call to this
helper is not generated at all.

In general, please add missing braces, read CODING_STYLE and use checkpatch.pl.

> +
>  acc   = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) |
>  ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO);
> -if (rs5_0 == 0) {
> -temp = acc;
> +
> +if (rs5_0 > 0) {
> +temp = acc >> MIPSDSP_ABS(rs5_0);
>  } else {
> -if (rs5_0 > 0) {
> -temp = acc >> rs5_0;
> -} else {
> -temp = acc << rs5_0;
> -}
> +temp = acc << MIPSDSP_ABS(rs5_0);
>  }
>
>  env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & MIPSDSP_LHI) >> 
> 32);
> diff --git a/tests/tcg/mips/mips32-dsp/shilo.c 
> b/tests/tcg/mips/mips32-dsp/shilo.c
> index b686616..ce8ebc6 100644
> --- a/tests/tcg/mips/mips32-dsp/shilo.c
> +++ b/tests/tcg/mips/mips32-dsp/shilo.c
> @@ -23,5 +23,23 @@ int main()
>  assert(ach == resulth);
>  assert(acl == resultl);
>
> +
> +ach = 0x1;
> +acl = 0x8000;
> +
> +resulth = 0x3;
> +resultl = 0x0;
> +
> +__asm
> +("mthi %0, $ac1\n\t"
> + "mtlo %1, $ac1\n\t"
> + "shilo $ac1, -1\n\t"
> + "mfhi %0, $ac1\n\t"
> + "mflo %1, $ac1\n\t"
> + : "+r"(ach), "+r"(acl)
> +);
> +assert(ach == resulth);
> +assert(acl == resultl);
> +
>  return 0;
>  }
> diff --git a/tests/tcg/mips/mips32-dsp/shilov.c 
> b/tests/tcg/mips/mips32-dsp/shilov.c
> index f186032..e1d6cea 100644
> --- a/tests/tcg/mips/mips32-dsp/shilov.c
> +++ b/tests/tcg/mips/mips32-dsp/shilov.c
> @@ -25,5 +25,25 @@ int main()
>  assert(ach == resulth);
>  assert(acl == resultl);
>
> +
> +rs  = 0x;
> +ach = 0x1;
> +acl = 0x8000;
> +
> +resulth = 0x3;
> +resultl = 0x0;
> +
> +__asm
> +("mthi %0, $ac1\n\t"
> + "mtlo %1, $ac1\n\t"
> + "shilov $ac1, %2\n\t"
> + "mfhi %0, $ac1\n\t"
> + "mflo %1, $ac1\n\t"
> + : "+r"(ach), "+r"(acl)
> + : "r"(rs)
> +);
> +assert(ach == resulth);
> +assert(acl == resultl);
> +
>  return 0;
>  }
> --
> 1.7.5.4
>
>



Re: [Qemu-devel] [RFC 6/8] qdev-properties.c: separate core from the code used only by qemu-system-*

2012-12-04 Thread Eduardo Habkost
On Tue, Dec 04, 2012 at 06:55:17PM +, Blue Swirl wrote:
> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost  wrote:
> > This separates the qdev properties code in two parts:
> >  - qdev-properties.c, that contains most of the qdev properties code;
> >  - qdev-properties-system.c for code specific for qemu-system-*,
> >containing:
> >- Property types: drive, chr, netdev, vlan, that depend on code that
> >  won't be included on *-user
> >- qemu_add_globals(), that depends on qemu-config.o.
> >
> > This change should help on two things:
> >  - Allowing DeviceState to be used by *-user without pulling
> >dependencies that are specific for qemu-system-*;
> >  - Writing qdev unit tests without pulling too many dependencies.
> >
> > The copyright/license header for the new file is directly copied from
> > qdev-properties.c.
> >
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Detailed changelog:
> >
> > Changes v1 (ehabkost) -> v2 (imammedo):
> >  - keep qdev_get_child_bus() in hw/qdev.c
> >  - put qdev_set_nic_properties() in hw/qdev-properties-system.c
> >
> > Changes v2 -> v3 (ehabkost):
> >  - updated the qdev_init_gpio_in() code on qdev-system.c to current
> >version
> >
> > Changes v3 -> v4 (ehabkost):
> >  - Added copyright/license information to qdev-properties-system.c
> >(based on copyright/license of qdev-properties.c)
> >  - Whitespace change at the end of qdev-properties.c
> >  - Don't create qdev-system.c, now we can keep the qdev.c code as-is
> >as the qdev.c dependencies were reduced
> >  - Rewrite patch description
> >
> > Changes v4 -> v5 (ehabkost):
> >  - Remove large copyright header and instead just point to the original
> >file it was based on
> >
> > Changes v5 -> v6 (ehabkost):
> >  - Removed inter-SoB line changelog from commit message
> > ---
> >  hw/Makefile.objs|   1 +
> >  hw/qdev-properties-system.c | 352 
> > 
> >  hw/qdev-properties.c| 321 +---
> >  hw/qdev-properties.h|   1 +
> >  hw/qdev.c   |  13 --
> >  5 files changed, 355 insertions(+), 333 deletions(-)
> >  create mode 100644 hw/qdev-properties-system.c
> >
> > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > index d581d8d..96a8365 100644
> > --- a/hw/Makefile.objs
> > +++ b/hw/Makefile.objs
> > @@ -185,6 +185,7 @@ common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o 
> > bt-hid.o
> >  common-obj-y += bt-hci-csr.o
> >  common-obj-y += msmouse.o ps2.o
> >  common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
> > +common-obj-y += qdev-properties-system.o
> >  common-obj-$(CONFIG_BRLAPI) += baum.o
> >
> >  # xen backend driver support
> > diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
> > new file mode 100644
> > index 000..9a7e0b3
> > --- /dev/null
> > +++ b/hw/qdev-properties-system.c
> > @@ -0,0 +1,352 @@
> > +/*
> > + * qdev property parsing and global properties
> > + * (parts specific for qemu-system-*)
> > + *
> > + * This file is based on code from hw/qdev-properties.c from
> > + * commit 4e68f7a0819f179c2ff90a60611806c789911cc2,
> > + * Copyright (c) Gerd Hoffmann  and other contributors.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "net.h"
> > +#include "qdev.h"
> > +#include "qerror.h"
> > +#include "blockdev.h"
> > +#include "hw/block-common.h"
> > +#include "net/hub.h"
> > +#include "qapi/qapi-visit-core.h"
> > +
> > +static void get_pointer(Object *obj, Visitor *v, Property *prop,
> > +const char *(*print)(void *ptr),
> > +const char *name, Error **errp)
> > +{
> > +DeviceState *dev = DEVICE(obj);
> > +void **ptr = qdev_get_prop_ptr(dev, prop);
> > +char *p;
> > +
> > +p = (char *) (*ptr ? print(*ptr) : "");
> > +visit_type_str(v, &p, name, errp);
> > +}
> > +
> > +static void set_pointer(Object *obj, Visitor *v, Property *prop,
> > +int (*parse)(DeviceState *dev, const char *str,
> > + void **ptr),
> > +const char *name, Error **errp)
> > +{
> > +DeviceState *dev = DEVICE(obj);
> > +Error *local_err = NULL;
> > +void **ptr = qdev_get_prop_ptr(dev, prop);
> > +char *str;
> > +int ret;
> > +
> > +if (dev->state != DEV_STATE_CREATED) {
> > +error_set(errp, QERR_PERMISSION_DENIED);
> > +return;
> > +}
> > +
> > +visit_type_str(v, &str, name, &local_err);
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return;
> > +}
> > +if (!*str) {
> > +g_free(str);
> > +*ptr = NULL;
> > +return;
> > +}
> > +ret = parse(dev, str, ptr);
> > +error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
> > +g_free(str);
> > +}
> > 

[Qemu-devel] [PATCH 1/6] target-i386/cpu.c: coding style fixes

2012-12-04 Thread Eduardo Habkost
- Use spaces instead of tabs on cpu_x86_cpuid().
- Use braces on 'if' statement cpu_x86_find_by_name().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c6c2ca0..7afe839 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1227,9 +1227,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 uint32_t minus_7_0_ebx_features = 0;
 uint32_t numvalue;
 
-for (def = x86_defs; def; def = def->next)
-if (name && !strcmp(name, def->name))
+for (def = x86_defs; def; def = def->next) {
+if (name && !strcmp(name, def->name)) {
 break;
+}
+}
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 kvm_cpu_fill_host(x86_cpu_def);
 } else if (!def) {
@@ -1835,17 +1837,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 }
 break;
 case 0x800A:
-   if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
-   *eax = 0x0001; /* SVM Revision */
-   *ebx = 0x0010; /* nr of ASIDs */
-   *ecx = 0;
-   *edx = env->cpuid_svm_features; /* optional features */
-   } else {
-   *eax = 0;
-   *ebx = 0;
-   *ecx = 0;
-   *edx = 0;
-   }
+if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+*eax = 0x0001; /* SVM Revision */
+*ebx = 0x0010; /* nr of ASIDs */
+*ecx = 0;
+*edx = env->cpuid_svm_features; /* optional features */
+} else {
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+}
 break;
 case 0xC000:
 *eax = env->cpuid_xlevel2;
-- 
1.7.11.7




[Qemu-devel] [PATCH 0/6] short x86 CPU init cleanup (v2)

2012-12-04 Thread Eduardo Habkost
Changes v1 -> v2:
 - Coding style changes (patches 1-2)
 - Use "return -1" directly instead of "goto error" on cpu_x86_find_by_name()
   (patch 2)

This series is based on the qom-cpu branch from afaerber's tree. The full
branch can be found in a git tree, at:
  git://github.com/ehabkost/qemu-hacks.git cpu-init-cleanup-short-v2
  https://github.com/ehabkost/qemu-hacks/tree/cpu-init-cleanup-short-v2

References to previous versions:
 v1: http://article.gmane.org/gmane.comp.emulators.qemu/182713
 previous (long) cleanup series:
   http://article.gmane.org/gmane.comp.emulators.qemu/180137


Eduardo Habkost (2):
  target-i386/cpu.c: coding style fixes
  target-i386: cpu: separate feature string parsing from CPU model
lookup

Igor Mammedov (4):
  target-i386: use define for cpuid vendor string size
  target-i386: postpone cpuid_level update to realize time
  add visitor for parsing hz[KMG] input string
  target-i386: use visit_type_hz to parse tsc_freq property value

 qapi/qapi-visit-core.c  |  11 +
 qapi/qapi-visit-core.h  |   2 +
 qapi/string-input-visitor.c |  22 ++
 target-i386/cpu.c   | 103 
 target-i386/cpu.h   |   2 +
 5 files changed, 102 insertions(+), 38 deletions(-)

-- 
1.7.11.7




[Qemu-devel] [PATCH 5/6] add visitor for parsing hz[KMG] input string

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Acked-by: Andreas Färber 
---
 qapi/qapi-visit-core.c  | 11 +++
 qapi/qapi-visit-core.h  |  2 ++
 qapi/string-input-visitor.c | 22 ++
 3 files changed, 35 insertions(+)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..5c8705e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,14 @@ void input_type_enum(Visitor *v, int *obj, const char 
*strings[],
 g_free(enum_str);
 *obj = value;
 }
+
+void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+if (!error_is_set(errp)) {
+if (v->type_freq) {
+v->type_freq(v, obj, name, errp);
+} else {
+v->type_int(v, obj, name, errp);
+}
+}
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..e5e7dd7 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -62,6 +62,7 @@ struct Visitor
 void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
 /* visit_type_size() falls back to (*type_uint64)() if type_size is unset 
*/
 void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
+void (*type_freq)(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
 };
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -91,5 +92,6 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char 
*name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error 
**errp);
+void visit_type_freq(Visitor *v, int64_t *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 497eb9a..74fe395 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool *present,
 *present = true;
 }
 
+static void parse_type_freq(Visitor *v, int64_t *obj, const char *name,
+Error **errp)
+{
+StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+char *endp = (char *) siv->string;
+long long val = 0;
+
+errno = 0;
+if (siv->string) {
+val = strtosz_suffix_unit(siv->string, &endp,
+ STRTOSZ_DEFSUFFIX_B, 1000);
+}
+if (!siv->string || val == -1 || *endp) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+  "a value representable as a non-negative int64");
+return;
+}
+
+*obj = val;
+}
+
 Visitor *string_input_get_visitor(StringInputVisitor *v)
 {
 return &v->visitor;
@@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char 
*str)
 v->visitor.type_str = parse_type_str;
 v->visitor.type_number = parse_type_number;
 v->visitor.start_optional = parse_start_optional;
+v->visitor.type_freq = parse_type_freq;
 
 v->string = str;
 return v;
-- 
1.7.11.7




[Qemu-devel] [PATCH 4/6] target-i386: postpone cpuid_level update to realize time

2012-12-04 Thread Eduardo Habkost
From: Igor Mammedov 

delay capping cpuid_level to 7 to realize time so property setters
for cpuid_7_0_ebx_features and "level" could be used in any order/time
between x86_cpu_initfn() and x86_cpu_realize().

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 86d7a61..a56a130 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1381,9 +1381,6 @@ static int cpu_x86_parse_featurestr(x86_def_t 
*x86_cpu_def, char *features)
 if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
 goto error;
 }
-if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
-x86_cpu_def->level = 7;
-}
 return 0;
 
 error:
@@ -2074,6 +2071,11 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
 void x86_cpu_realize(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+
+if (env->cpuid_7_0_ebx_features && env->cpuid_level < 7) {
+env->cpuid_level = 7;
+}
 
 #ifndef CONFIG_USER_ONLY
 qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH] virtfs-proxy-helper: check return code of setfsgid/setfsuid

2012-12-04 Thread Aneesh Kumar K.V
Paolo Bonzini  writes:

> Il 11/10/2012 09:25, M. Mohan Kumar ha scritto:
>> Also as per the man page:
>>When glibc determines that the argument is not a valid user ID,
>>it will return -1 and set errno  to  EINVAL
>>without attempting the system call.
>> 
>> If it mean a nonexistent id by 'not a valid user ID' it may be a
>> problem in virtfs case.
>
> I think only -1 would be an invalid user ID, or perhaps a user ID >
> 65535 if the kernel only supports 16-bit user IDs.
>
> Rather than dealing with the kernel, can we just use
> setresuid/setresgid like in the following (untested) patch?
>
> Paolo
>
> ps: so far in my short life I had managed to stay away from privilege
> dropping, so please review with extra care.
>
> --- 8< ---
> From: Paolo Bonzini 
> Date: Thu, 11 Oct 2012 14:20:23 +0200
> Subject: [PATCH] virtfs-proxy-helper: use setresuid and setresgid
>
> The setfsuid and setfsgid system calls are obscure and they complicate
> the error checking (that glibc's warn_unused_result "feature" forces
> us to do).  Switch to the standard setresuid and setresgid functions.
>
> ---
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index f9a8270..07b3b5b 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -272,31 +272,76 @@ static int send_status(int sockfd, struct iovec *iovec, 
> int status)
>  /*
>   * from man 7 capabilities, section
>   * Effect of User ID Changes on Capabilities:
> - * 4. If the file system user ID is changed from 0 to nonzero (see 
> setfsuid(2))
> - * then the following capabilities are cleared from the effective set:
> - * CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH,  CAP_FOWNER, CAP_FSETID,
> - * CAP_LINUX_IMMUTABLE  (since  Linux 2.2.30), CAP_MAC_OVERRIDE, and 
> CAP_MKNOD
> - * (since Linux 2.2.30). If the file system UID is changed from nonzero to 0,
> - * then any of these capabilities that are enabled in the permitted set
> - * are enabled in the effective set.
> + * If the effective user ID is changed from nonzero to 0, then the permitted
> + * set is copied to the effective set.  If the effective user ID is changed
> + * from 0 to nonzero, then all capabilities are are cleared from the 
> effective
> + * set.
> + *
> + * The setfsuid/setfsgid man pages warn that changing the effective user ID 
> may
> + * expose the program to unwanted signals, but this is not true anymore: for 
> an
> + * unprivileged (without CAP_KILL) program to send a signal, the real or
> + * effective user ID of the sending process must equal the real or saved user
> + * ID of the target process.  Even when dropping privileges, it is enough to
> + * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
> + * be exposed to signals.  So just use setresuid/setresgid.
>   */
> -static int setfsugid(int uid, int gid)
> +static int setugid(int uid, int gid, int *suid, int *sgid)
>  {
> +int retval;
> +
>  /*
> - * We still need DAC_OVERRIDE because  we don't change
> + * We still need DAC_OVERRIDE because we don't change
>   * supplementary group ids, and hence may be subjected DAC rules
>   */
>  cap_value_t cap_list[] = {
>  CAP_DAC_OVERRIDE,
>  };
>
> -setfsgid(gid);
> -setfsuid(uid);
> +/*
> + * If suid/sgid are NULL, the saved uid/gid is set to the
> + * new effective uid/gid.  If they are not, the saved uid/gid
> + * is set to the current effective user id and stored into
> + * *suid and *sgid.
> + */
> +if (!suid) {
> +suid = &uid;
> +} else {
> +*suid = geteuid();
> +}
> +if (!sgid) {
> +sgid = &gid;
> +} else {
> +*sgid = getegid();
> +}
> +

I found this to be confusing. How about avoiding all those pointers, something
like below ? If you are ok can I add the signed-off-by for this ? I can
test this and get a pull request out with the build fix.

commit 24cc9f0d07c2a505bfafbdcb72006f2eda1288a4
Author: Paolo Bonzini 
Date:   Thu Oct 11 14:20:23 2012 +0200

virtfs-proxy-helper: use setresuid and setresgid

The setfsuid and setfsgid system calls are obscure and they complicate
the error checking (that glibc's warn_unused_result "feature" forces
us to do).  Switch to the standard setresuid and setresgid functions.

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index f9a8270..49ab0eb 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -272,31 +272,59 @@ static int send_status(int sockfd, struct iovec *iovec, 
int status)
 /*
  * from man 7 capabilities, section
  * Effect of User ID Changes on Capabilities:
- * 4. If the file system user ID is changed from 0 to nonzero (see setfsuid(2))
- * then the following capabilities are cleared from the effective set:
- * CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH,  CAP_FOWNER, CAP_FSETID,
- * CAP_LINUX_IMMUTABLE  (since  Li

  1   2   3   >