Re: [Qemu-devel] Memory API: handling unassigned physical memory
On Mon, Apr 30, 2012 at 14:33, Mark Cave-Ayland wrote: > On 30/04/12 15:03, Peter Maydell wrote: > >>> Therefore I can't change it to my (modified) sbus_mmio_map() function >>> because it would break other non-SPARC platforms, and AIUI there is >>> nothing >>> in the memory API that allows me to move a subregion to a different >>> MemoryRegion parent, even if I can get a reference to it with >>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I >>> misunderstood something? >> >> >> Init functions like esp_init should be purely convenience functions >> which create, set properties on, and map the sysbus/qdev device. If >> they make use of private knowledge about the internals of the device >> then this is wrong and should be fixed. For esp_init() it looks like >> the handling of dma_memory_read, dma_memory_write, dma_opaque, it_shift >> and dma_enabled are wrong. >> >> If you fix that then you can just ignore the convenience function, >> and create, configure and map the device as appropriate for you. > > > Right I think I'm starting to understand this now - in which case it becomes > a matter of just copying a handful of lines within sun4m which is more > bearable. > > In your view, would a suitable fix be to change dma_memory_read, > dma_memory_write, dma_opaque, it_shift and dma_enabled to be qdev properties > and modify esp_init() to return the qdev reference so they can be set by the > caller? There's an ongoing work to introduce IOMMUs by changing how DMA work and this could simplify the DMA part. There's no clean way to use function pointers in qdev. For it_shift, a qdev or QOM property should be OK. The signal dma_enabled should be eventually replaced by a Pin. > > > ATB, > > Mark. >
[Qemu-devel] [PATCH v7 0/5] add i.MX31 support
This patch series adds rudimentary support for the Freescale i.MX31 SoC, and the Kyoto Micro KZM-ARM11-01, an evaluation board built around the Freescale i.MX31. Changes since last patch round: (v6) * shortened patch titles * canonised spelling of `Freescale' * Fixed 10 to 32-bit sign extension in the clock-control-module * Use a post-load hook in the clock-control-module instead of storing frequencies in the VMState. * Deleted undocumented second UART on KZM board's FPGA. Apart from that the patches are unchanged. Thanks to Peter M and Andreas F for comments. -- Dr Peter Chubbwww.nicta.com.au peter DOT chubb AT nicta.com.au http://www.ertos.nicta.com.au ERTOS within National ICT Australia >From Imagination to Impact Imagining the (ICT) Future
[Qemu-devel] [PATCH v7 5/5] i.MX31: KZM-ARM11-01 evaluation board
Board support for Kyoto Micro's KZM-ARM11-01, an evaluation board built around the Freescale i.MX31. Signed-off-by: Philip O'Sullivan Signed-off-by: Peter Chubb --- Makefile.target |2 hw/kzm.c| 155 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 hw/kzm.c Index: qemu-working/hw/kzm.c === --- /dev/null +++ qemu-working/hw/kzm.c @@ -0,0 +1,155 @@ +/* + * KZM Board System emulation. + * + * Copyright (c) 2008 OKL and 2011 NICTA + * Written by Hans + * Updated by Peter Chubb. + * + * This code is licenced under the GPL, version 2 or later. + * See the file `COPYING' in the top level directory. + * + * It (partially) emulates a Kyoto Microcomputer + * KZM-ARM11-01 evaluation board, with a Freescale + * I.MX31 SoC + */ + +#include "sysbus.h" +#include "exec-memory.h" +#include "hw.h" +#include "arm-misc.h" +#include "devices.h" +#include "net.h" +#include "sysemu.h" +#include "boards.h" +#include "pc.h" /* for the FPGA UART that emulates a 16550 */ +#include "imx.h" + +/* Memory map for Kzm Emulation Baseboard: + * 0x-0x3fff 16k secure ROM IGNORED + * 0x4000-0x00407fff Reserved IGNORED + * 0x00404000-0x00407fff ROM IGNORED + * 0x00408000-0x0fff Reserved IGNORED + * 0x1000-0x1fffbfff RAM aliasing IGNORED + * 0x1fffc000-0x1fff RAM EMULATED + * 0x2000-0x2fff Reserved IGNORED + * 0x3000-0x7fff I.MX31 Internal Register Space + * 0x43f0 IO_AREA0 + * 0x43f9 UART1 EMULATED + * 0x43f94000 UART2 EMULATED + * 0x6800 AVIC EMULATED + * 0x53f8 CCM EMULATED + * 0x53f94000 PIT 1 EMULATED + * 0x53f98000 PIT 2 EMULATED + * 0x53f9 GPT EMULATED + * 0x8000-0x87ff RAM EMULATED + * 0x8800-0x8fff RAM Aliasing EMULATED + * 0xa000-0xafff NAND Flash IGNORED + * 0xb000-0xb3ff Unavailable IGNORED + * 0xb400-0xb4000fff 8-bit free space IGNORED + * 0xb4001000-0xb400100f Board controlIGNORED + * 0xb4001003 DIP switch + * 0xb4001010-0xb400101f 7-segment LEDIGNORED + * 0xb4001020-0xb400102f LED IGNORED + * 0xb4001030-0xb400103f LED IGNORED + * 0xb4001040-0xb400104f FPGA, UART EMULATED + * 0xb4001050-0xb400105f FPGA, UART EMULATED + * 0xb4001060-0xb40f FPGA IGNORED + * 0xb600-0xb61f LAN controller EMULATED + * 0xb620-0xb62f FPGA NAND Controller IGNORED + * 0xb630-0xb7ff Free IGNORED + * 0xb800-0xb8004fff Memory control registers IGNORED + * 0xc000-0xc3ff PCMCIA/CFIGNORED + * 0xc400-0x Reserved IGNORED + */ + +#define KZM_RAMADDRESS (0x8000) +#define KZM_FPGA (0xb4001040) + +static struct arm_boot_info kzm_binfo = { +.loader_start = KZM_RAMADDRESS, +.board_id = 1722, +}; + +static void kzm_init(ram_addr_t ram_size, + const char *boot_device, + const char *kernel_filename, const char *kernel_cmdline, + const char *initrd_filename, const char *cpu_model) +{ +CPUARMState *env; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *sram = g_new(MemoryRegion, 1); +MemoryRegion *ram_alias = g_new(MemoryRegion, 1); +qemu_irq *cpu_pic; +DeviceState *dev; +DeviceState *ccm; + +if (!cpu_model) { +cpu_model = "arm1136"; +} + +env = cpu_init(cpu_model); +if (!env) { +fprintf(stderr, "Unable to find CPU definition\n"); +exit(1); +} + +/* On a real system, the first 16k is a `secure boot rom' */ + +memory_region_init_ram(ram, "kzm.ram", ram_size); +vmstate_register_ram_global(ram); +memory_region_add_subregion(address_space_mem, KZM_RAMADDRESS, ram); + +memory_region_init_alias(ram_alias, "ram.alias", ram, 0, ram_size); +memory_region_add_subregion(address_space_mem, 0x8800, ram_alias); + +memory_region_init_ram(sram, "kzm.sram", 0x4000); +memory_region_add_subregion(address_space_mem, 0x1FFFC000, sram); + + +cpu_pic = arm_pic_init_cpu(env); +dev = sysbus_create_varargs("imx_avic", 0x6800, + cpu_pic[ARM_PIC_CPU_IRQ], + cpu_pic[ARM_PIC_CPU_FIQ], NULL); + + +imx_serial_create(0, 0x43f9, qdev_get_gpio_in(dev, 45)); +imx_ser
[Qemu-devel] [PATCH v7 4/5] i.MX31: Interrupt Controller
Implement the Freescale i.MX31 advanced vectored interrupt controller, at least to the extent it is used by Linux 3.x Vectors are not implemented. Signed-off-by: Philip O'Sullivan Signed-off-by: Peter Chubb Reviewed-by: Peter Maydell --- Makefile.target |2 hw/imx_avic.c | 409 2 files changed, 410 insertions(+), 1 deletion(-) create mode 100644 hw/imx_avic.c Index: qemu-working/hw/imx_avic.c === --- /dev/null +++ qemu-working/hw/imx_avic.c @@ -0,0 +1,409 @@ +/* + * IMX31 Vectored Interrupt Controller + * + * Note this is NOT the PL192 provided by ARM, but + * a custom implementation by Freescale. + * + * Copyright (c) 2008 OKL + * Copyright (c) 2011 NICTA Pty Ltd + * Originally Written by Hans Jiang + * + * This code is licenced under the GPL version 2 or later. See + * the COPYING file in the top-level directory. + * + * TODO: implement vectors. + */ + +#include "hw.h" +#include "sysbus.h" +#include "host-utils.h" + +#define DEBUG_INT 1 +#undef DEBUG_INT /* comment out for debugging */ + +#ifdef DEBUG_INT +#define DPRINTF(fmt, args...) \ +do { printf("imx_avic: " fmt , ##args); } while (0) +#else +#define DPRINTF(fmt, args...) do {} while (0) +#endif + +/* + * Define to 1 for messages about attempts to + * access unimplemented registers or similar. + */ +#define DEBUG_IMPLEMENTATION 1 +#if DEBUG_IMPLEMENTATION +# define IPRINTF(fmt, args...) \ +do { fprintf(stderr, "imx_avic: " fmt, ##args); } while (0) +#else +# define IPRINTF(fmt, args...) do {} while (0) +#endif + +#define IMX_AVIC_NUM_IRQS 64 + +/* Interrupt Control Bits */ +#define ABFLAG (1<<25) +#define ABFEN (1<<24) +#define NIDIS (1<<22) /* Normal Interrupt disable */ +#define FIDIS (1<<21) /* Fast interrupt disable */ +#define NIAD (1<<20) /* Normal Interrupt Arbiter Rise ARM level */ +#define FIAD (1<<19) /* Fast Interrupt Arbiter Rise ARM level */ +#define NM(1<<18) /* Normal interrupt mode */ + + +#define PRIO_PER_WORD (sizeof(uint32_t) * 8 / 4) +#define PRIO_WORDS (IMX_AVIC_NUM_IRQS/PRIO_PER_WORD) + +typedef struct { +SysBusDevice busdev; +MemoryRegion iomem; +uint64_t pending; +uint64_t enabled; +uint64_t is_fiq; +uint32_t intcntl; +uint32_t intmask; +qemu_irq irq; +qemu_irq fiq; +uint32_t prio[PRIO_WORDS]; /* Priorities are 4-bits each */ +} IMXAVICState; + +static const VMStateDescription vmstate_imx_avic = { +.name = "imx-avic", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField[]) { +VMSTATE_UINT64(pending, IMXAVICState), +VMSTATE_UINT64(enabled, IMXAVICState), +VMSTATE_UINT64(is_fiq, IMXAVICState), +VMSTATE_UINT32(intcntl, IMXAVICState), +VMSTATE_UINT32(intmask, IMXAVICState), +VMSTATE_UINT32_ARRAY(prio, IMXAVICState, PRIO_WORDS), +VMSTATE_END_OF_LIST() +}, +}; + + + +static inline int imx_avic_prio(IMXAVICState *s, int irq) +{ +uint32_t word = irq / PRIO_PER_WORD; +uint32_t part = 4 * (irq % PRIO_PER_WORD); +return 0xf & (s->prio[word] >> part); +} + +static inline void imx_avic_set_prio(IMXAVICState *s, int irq, int prio) +{ +uint32_t word = irq / PRIO_PER_WORD; +uint32_t part = 4 * (irq % PRIO_PER_WORD); +uint32_t mask = ~(0xf << part); +s->prio[word] &= mask; +s->prio[word] |= prio << part; +} + +/* Update interrupts. */ +static void imx_avic_update(IMXAVICState *s) +{ +int i; +uint64_t new = s->pending & s->enabled; +uint64_t flags; + +flags = new & s->is_fiq; +qemu_set_irq(s->fiq, !!flags); + +flags = new & ~s->is_fiq; +if (!flags || (s->intmask == 0x1f)) { +qemu_set_irq(s->irq, !!flags); +return; +} + +/* + * Take interrupt if there's a pending interrupt with + * priority higher than the value of intmask + */ +for (i = 0; i < IMX_AVIC_NUM_IRQS; i++) { +if (flags & (1UL << i)) { +if (imx_avic_prio(s, i) > s->intmask) { +qemu_set_irq(s->irq, 1); +return; +} +} +} +qemu_set_irq(s->irq, 0); +} + +static void imx_avic_set_irq(void *opaque, int irq, int level) +{ +IMXAVICState *s = (IMXAVICState *)opaque; + +if (level) { +DPRINTF("Raising IRQ %d, prio %d\n", +irq, imx_avic_prio(s, irq)); +s->pending |= (1ULL << irq); +} else { +DPRINTF("Clearing IRQ %d, prio %d\n", +irq, imx_avic_prio(s, irq)); +s->pending &= ~(1ULL << irq); +} + +imx_avic_update(s); +} + + +static uint64_t imx_avic_read(void *opaque, + target_phys_addr_t offset, unsigned size) +{ +IMXAVICState *s = (IMXAVICState *)opaque; + + +DPRINTF("read(offset = 0x%x)\n", offset >> 2); +switch (offset >> 2) { +case 0: /* INTCNTL */ +return s->intcntl; +
[Qemu-devel] [PATCH v7 2/5] i.MX31: Clock Control Module
For Linux to be able to work out how fast its clocks are going, so that timer ticks come approximately at the right time, it needs to be able to query the clock control module (CCM). This is the start of a CCM implementation. It currently knows only about the MCU, HSP and IPG clocks --- i.e., the ones used to feed the periodic and general purpose timers. Signed-off-by: Peter Chubb --- Makefile.target |2 hw/imx.h| 10 + hw/imx_ccm.c| 321 3 files changed, 332 insertions(+), 1 deletion(-) Index: qemu-working/Makefile.target === --- qemu-working.orig/Makefile.target +++ qemu-working/Makefile.target @@ -402,7 +402,7 @@ obj-arm-y += vexpress.o obj-arm-y += strongarm.o obj-arm-y += collie.o obj-arm-y += pl041.o lm4549.o -obj-arm-y += imx_serial.o +obj-arm-y += imx_serial.o imx_ccm.o obj-arm-$(CONFIG_FDT) += device_tree.o obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o Index: qemu-working/hw/imx_ccm.c === --- /dev/null +++ qemu-working/hw/imx_ccm.c @@ -0,0 +1,321 @@ +/* + * IMX31 Clock Control Module + * + * Copyright (C) 2012 NICTA + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * To get the timer frequencies right, we need to emulate at least part of + * the CCM. + */ + +#include "hw.h" +#include "sysbus.h" +#include "sysemu.h" +#include "imx.h" + +#define CKIH_FREQ 2600 /* 26MHz crystal input */ +#define CKIL_FREQ32768 /* nominal 32khz clock */ + + +//#define DEBUG_CCM 1 +#ifdef DEBUG_CCM +#define DPRINTF(fmt, args...) \ +do { printf("imx_ccm: " fmt , ##args); } while (0) +#else +#define DPRINTF(fmt, args...) do {} while (0) +#endif + +static int imx_ccm_post_load(void *opaque, int version_id); + +typedef struct { +SysBusDevice busdev; +MemoryRegion iomem; + +uint32_t ccmr; +uint32_t pdr0; +uint32_t pdr1; +uint32_t mpctl; +uint32_t spctl; +uint32_t cgr[3]; +uint32_t pmcr0; +uint32_t pmcr1; + +/* Frequencies precalculated on register changes */ +uint32_t pll_refclk_freq; +uint32_t mcu_clk_freq; +uint32_t hsp_clk_freq; +uint32_t ipg_clk_freq; +} IMXCCMState; + +static const VMStateDescription vmstate_imx_ccm = { +.name = "imx-ccm", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField[]) { +VMSTATE_UINT32(ccmr, IMXCCMState), +VMSTATE_UINT32(pdr0, IMXCCMState), +VMSTATE_UINT32(pdr1, IMXCCMState), +VMSTATE_UINT32(mpctl, IMXCCMState), +VMSTATE_UINT32(spctl, IMXCCMState), +VMSTATE_UINT32_ARRAY(cgr, IMXCCMState, 3), +VMSTATE_UINT32(pmcr0, IMXCCMState), +VMSTATE_UINT32(pmcr1, IMXCCMState), +VMSTATE_UINT32(pll_refclk_freq, IMXCCMState), +}, +.post_load = imx_ccm_post_load, +}; + +/* CCMR */ +#define CCMR_FPME (1<<0) +#define CCMR_MPE (1<<3) +#define CCMR_MDS (1<<7) +#define CCMR_FPMF (1<<26) +#define CCMR_PRCS (3<<1) + +/* PDR0 */ +#define PDR0_MCU_PODF_SHIFT (0) +#define PDR0_MCU_PODF_MASK (0x7) +#define PDR0_MAX_PODF_SHIFT (3) +#define PDR0_MAX_PODF_MASK (0x7) +#define PDR0_IPG_PODF_SHIFT (6) +#define PDR0_IPG_PODF_MASK (0x3) +#define PDR0_NFC_PODF_SHIFT (8) +#define PDR0_NFC_PODF_MASK (0x7) +#define PDR0_HSP_PODF_SHIFT (11) +#define PDR0_HSP_PODF_MASK (0x7) +#define PDR0_PER_PODF_SHIFT (16) +#define PDR0_PER_PODF_MASK (0x1f) +#define PDR0_CSI_PODF_SHIFT (23) +#define PDR0_CSI_PODF_MASK (0x1ff) + +#define EXTRACT(value, name) (((value) >> PDR0_##name##_PODF_SHIFT) \ + & PDR0_##name##_PODF_MASK) +#define INSERT(value, name) (((value) & PDR0_##name##_PODF_MASK) << \ + PDR0_##name##_PODF_SHIFT) +/* PLL control registers */ +#define PD(v) (((v) >> 26) & 0xf) +#define MFD(v) (((v) >> 16) & 0x3ff) +#define MFI(v) (((v) >> 10) & 0xf); +#define MFN(v) ((v) & 0x3ff) + +#define PLL_PD(x) (((x) & 0xf) << 26) +#define PLL_MFD(x) (((x) & 0x3ff) << 16) +#define PLL_MFI(x) (((x) & 0xf) << 10) +#define PLL_MFN(x) (((x) & 0x3ff) << 0) + +uint32_t imx_clock_frequency(DeviceState *dev, IMXClk clock) +{ +IMXCCMState *s = container_of(dev, IMXCCMState, busdev.qdev); + +switch (clock) { +case NOCLK: +return 0; +case MCU: +return s->mcu_clk_freq; +case HSP: +return s->hsp_clk_freq; +case IPG: +return s->ipg_clk_freq; +case CLK_32k: +return CKIL_FREQ; +} +return 0; +} + +/* + * Calculate PLL output frequency + */ +static uint32_t calc_pll(uint32_t pllreg, uint32_t base_freq) +{ +int32_t mfn = MFN(pllreg); /* Numerator */ +uint32_t mfi = MFI(pllreg); /* Integer part */ +uint32_t mfd = 1 + MFD(pllreg); /* Denomin
Re: [Qemu-devel] [Bug 992067] Re: Windows 2008R2 very slow cold boot when >4GB memory
On Mon, Apr 30, 2012 at 06:07:55PM -, Anthony Liguori wrote: > This should be resolved by using Hyper-V relaxed timers which is in the > latest development version of QEMU. You would need to add -cpu > host,+hv_relaxed to the command line to verify this. > The described scenario still shouldn't happen and it doesn't for me. Is this 32 or 64 bit version of Windows. Are you running 32 bit version of the host may be (but Scientific Linux shouldn't have that)? What is your command line? > ** Changed in: qemu >Status: New => Fix Committed > > -- > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. > https://bugs.launchpad.net/bugs/992067 > > Title: > Windows 2008R2 very slow cold boot when >4GB memory > > Status in QEMU: > Fix Committed > > Bug description: > I've been having a consistent problem booting 2008R2 guests with > 4096MB of RAM or greater. On the initial boot the KVM process starts > out with a ~200MB memory allocation and will use 100% of all CPU > allocated to it. The RES memory of the KVM process slowly rises by > around 200mb every few minutes until it reaches it's memory allocation > (several hours in some cases). Whilst this is happening the guest will > usually blue screen with the message of - > > A clock interrupt was not received on a secondary processor within the > allocated time interval > > If I let the KVM process continue to run it will eventually allocate > the required memory the guest will run at full speed, usually > restarting after the blue screen and booting into startup repair. From > here you can restart it and it will boot perfectly. Once booted the > guest has no performance issues at all. > > I've tried everything I could think of. Removing PAE, playing with > huge pages, different kernels, different userspaces, different > systems, different backing file systems, different processor feature > set, with or without Virtio etc. My best theory is that the problem is > caused by Windows 2008 zeroing out all the memory on boot and > something is causing this to be held up or slowed to a crawl. The > hosts always have memory free to boot the guest and are not using swap > at all. > > Nothing so far has solved the issue. A few observations I've made about the > issue are - > Large memory 2008R2 guests seem to boot fine (or with a small delay) when > they are the first to boot on the host after a reboot > Sometimes dropping the disk cache (echo 1 > /proc/sys/vm/drop_caches) will > cause them to boot faster > > > The hosts I've tried are - > All Nehalem based (5540, 5620 and 5660) > Host ram of 48GB, 96GB and 192GB > Storage on NFS, Gluster and local (ext4, xfs and zfs) > QED, QCOW and RAW formats > Scientific Linux 6.1 with the standard kernel 2.6.32, 2.6.38 and 3.3.1 > KVM userspaces 0.12, 0.14 and (currently) 0.15.1 > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/992067/+subscriptions -- Gleb.
[Qemu-devel] [PATCH v7 3/5] i.MX31: Timers
Implement the timers on the Freescale i.MX31 SoC. This is not a complete implementation, but gives enough for Linux to boot and run. In particular external triggers, which are not useful under QEMU, are not implemented. Signed-off-by: Philip O'Sullivan Signed-off-by: Peter Chubb Reviewed-by: Peter Maydell --- Makefile.target |2 hw/imx.h|8 hw/imx_timer.c | 689 3 files changed, 698 insertions(+), 1 deletion(-) create mode 100644 hw/imx_timer.c Index: qemu-working/hw/imx_timer.c === --- /dev/null +++ qemu-working/hw/imx_timer.c @@ -0,0 +1,689 @@ +/* + * IMX31 Timer + * + * Copyright (c) 2008 OK Labs + * Copyright (c) 2011 NICTA Pty Ltd + * Originally Written by Hans Jiang + * Updated by Peter Chubb + * + * This code is licenced under GPL version 2 or later. See + * the COPYING file in the top-level directory. + * + */ + +#include "hw.h" +#include "qemu-timer.h" +#include "ptimer.h" +#include "sysbus.h" +#include "imx.h" + +//#define DEBUG_TIMER 1 +#ifdef DEBUG_TIMER +# define DPRINTF(fmt, args...) \ + do { printf("imx_timer: " fmt , ##args); } while (0) +#else +# define DPRINTF(fmt, args...) do {} while (0) +#endif + +/* + * Define to 1 for messages about attempts to + * access unimplemented registers or similar. + */ +#define DEBUG_IMPLEMENTATION 1 +#if DEBUG_IMPLEMENTATION +# define IPRINTF(fmt, args...) \ +do { fprintf(stderr, "imx_timer: " fmt, ##args); } while (0) +#else +# define IPRINTF(fmt, args...) do {} while (0) +#endif + +/* + * GPT : General purpose timer + * + * This timer counts up continuously while it is enabled, resetting itself + * to 0 when it reaches TIMER_MAX (in freerun mode) or when it + * reaches the value of ocr1 (in periodic mode). WE simulate this using a + * QEMU ptimer counting down from ocr1 and reloading from ocr1 in + * periodic mode, or counting from ocr1 to zero, then TIMER_MAX - ocr1. + * waiting_rov is set when counting from TIMER_MAX. + * + * In the real hardware, there are three comparison registers that can + * trigger interrupts, and compare channel 1 can be used to + * force-reset the timer. However, this is a `bare-bones' + * implementation: only what Linux 3.x uses has been implemented + * (free-running timer from 0 to OCR1 or TIMER_MAX) . + */ + + +#define TIMER_MAX 0XUL + +/* Control register. Not all of these bits have any effect (yet) */ +#define GPT_CR_EN (1 << 0) /* GPT Enable */ +#define GPT_CR_ENMOD (1 << 1) /* GPT Enable Mode */ +#define GPT_CR_DBGEN (1 << 2) /* GPT Debug mode enable */ +#define GPT_CR_WAITEN (1 << 3) /* GPT Wait Mode Enable */ +#define GPT_CR_DOZEN (1 << 4) /* GPT Doze mode enable */ +#define GPT_CR_STOPEN (1 << 5) /* GPT Stop Mode Enable */ +#define GPT_CR_CLKSRC_SHIFT (6) +#define GPT_CR_CLKSRC_MASK (0x7) + +#define GPT_CR_FRR(1 << 9) /* Freerun or Restart */ +#define GPT_CR_SWR(1 << 15) /* Software Reset */ +#define GPT_CR_IM1(3 << 16) /* Input capture channel 1 mode (2 bits) */ +#define GPT_CR_IM2(3 << 18) /* Input capture channel 2 mode (2 bits) */ +#define GPT_CR_OM1(7 << 20) /* Output Compare Channel 1 Mode (3 bits) */ +#define GPT_CR_OM2(7 << 23) /* Output Compare Channel 2 Mode (3 bits) */ +#define GPT_CR_OM3(7 << 26) /* Output Compare Channel 3 Mode (3 bits) */ +#define GPT_CR_FO1(1 << 29) /* Force Output Compare Channel 1 */ +#define GPT_CR_FO2(1 << 30) /* Force Output Compare Channel 2 */ +#define GPT_CR_FO3(1 << 31) /* Force Output Compare Channel 3 */ + +#define GPT_SR_OF1 (1 << 0) +#define GPT_SR_ROV (1 << 5) + +#define GPT_IR_OF1IE (1 << 0) +#define GPT_IR_ROVIE (1 << 5) + +typedef struct { +SysBusDevice busdev; +ptimer_state *timer; +MemoryRegion iomem; +DeviceState *ccm; + +uint32_t cr; +uint32_t pr; +uint32_t sr; +uint32_t ir; +uint32_t ocr1; +uint32_t cnt; + +uint32_t waiting_rov; +qemu_irq irq; +} IMXTimerGState; + +static const VMStateDescription vmstate_imx_timerg = { +.name = "imx-timerg", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField[]) { +VMSTATE_UINT32(cr, IMXTimerGState), +VMSTATE_UINT32(pr, IMXTimerGState), +VMSTATE_UINT32(sr, IMXTimerGState), +VMSTATE_UINT32(ir, IMXTimerGState), +VMSTATE_UINT32(ocr1, IMXTimerGState), +VMSTATE_UINT32(cnt, IMXTimerGState), +VMSTATE_UINT32(waiting_rov, IMXTimerGState), +VMSTATE_PTIMER(timer, IMXTimerGState), +VMSTATE_END_OF_LIST() +} +}; + +static const IMXClk imx_timerg_clocks[] = { +NOCLK,/* 000 No clock source */ +IPG, /* 001 ipg_clk, 532MHz*/ +IPG, /* 010 ipg_clk_highfreq */ +NOCLK,/* 011 not defined */ +CLK_32k, /* 100 ipg_clk_32k */ +NOCLK,/* 101 not defi
[Qemu-devel] [PATCH v7 1/5] i.MX: UART support
Implement the Freescale i.MX UART. This uart is used in a variety of SoCs, including some by Motorola, as well as in the Freescale i.MX series. This patch gives only a `bare-bones' implementation, enough to run Linux or OKL4, but that's about it. Signed-off-by: Philip O'Sullivan Signed-off-by: Peter Chubb Reviewed-by: Peter Maydell --- Makefile.target |1 hw/imx.h| 16 + hw/imx_serial.c | 467 3 files changed, 484 insertions(+) create mode 100644 hw/imx_serial.c Index: qemu-working/hw/imx_serial.c === --- /dev/null +++ qemu-working/hw/imx_serial.c @@ -0,0 +1,467 @@ +/* + * IMX31 UARTS + * + * Copyright (c) 2008 OKL + * Originally Written by Hans Jiang + * Copyright (c) 2011 NICTA Pty Ltd. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * This is a `bare-bones' implementation of the IMX series serial ports. + * TODO: + * -- implement FIFOs. The real hardware has 32 word transmit + * and receive FIFOs; we currently use a 1-char buffer + * -- implement DMA + * -- implement BAUD-rate and modem lines, for when the backend + * is a real serial device. + */ + +#include "hw.h" +#include "sysbus.h" +#include "sysemu.h" +#include "qemu-char.h" +#include "imx.h" + +//#define DEBUG_SERIAL 1 +#ifdef DEBUG_SERIAL +#define DPRINTF(fmt, args...) \ +do { printf("imx_serial: " fmt , ##args); } while (0) +#else +#define DPRINTF(fmt, args...) do {} while (0) +#endif + +/* + * Define to 1 for messages about attempts to + * access unimplemented registers or similar. + */ +//#define DEBUG_IMPLEMENTATION 1 +#ifdef DEBUG_IMPLEMENTATION +# define IPRINTF(fmt, args...) \ +do { fprintf(stderr, "imx_serial: " fmt, ##args); } while (0) +#else +# define IPRINTF(fmt, args...) do {} while (0) +#endif + +typedef struct { +SysBusDevice busdev; +MemoryRegion iomem; +int32_t readbuff; + +uint32_t usr1; +uint32_t usr2; +uint32_t ucr1; +uint32_t ucr2; +uint32_t uts1; + +/* + * The registers below are implemented just so that the + * guest OS sees what it has written + */ +uint32_t onems; +uint32_t ufcr; +uint32_t ubmr; +uint32_t ubrc; +uint32_t ucr3; + +qemu_irq irq; +CharDriverState *chr; +} IMXSerialState; + +static const VMStateDescription vmstate_imx_serial = { +.name = "imx-serial", +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField[]) { +VMSTATE_INT32(readbuff, IMXSerialState), +VMSTATE_UINT32(usr1, IMXSerialState), +VMSTATE_UINT32(usr2, IMXSerialState), +VMSTATE_UINT32(ucr1, IMXSerialState), +VMSTATE_UINT32(uts1, IMXSerialState), +VMSTATE_UINT32(onems, IMXSerialState), +VMSTATE_UINT32(ufcr, IMXSerialState), +VMSTATE_UINT32(ubmr, IMXSerialState), +VMSTATE_UINT32(ubrc, IMXSerialState), +VMSTATE_UINT32(ucr3, IMXSerialState), +VMSTATE_END_OF_LIST() +}, +}; + + +#define URXD_CHARRDY(1<<15) /* character read is valid */ +#define URXD_ERR(1<<14) /* Character has error */ +#define URXD_BRK(1<<11) /* Break received */ + +#define USR1_PARTYER(1<<15) /* Parity Error */ +#define USR1_RTSS (1<<14) /* RTS pin status */ +#define USR1_TRDY (1<<13) /* Tx ready */ +#define USR1_RTSD (1<<12) /* RTS delta: pin changed state */ +#define USR1_ESCF (1<<11) /* Escape sequence interrupt */ +#define USR1_FRAMERR(1<<10) /* Framing error */ +#define USR1_RRDY (1<<9)/* receiver ready */ +#define USR1_AGTIM (1<<8)/* Aging timer interrupt */ +#define USR1_DTRD (1<<7)/* DTR changed */ +#define USR1_RXDS (1<<6)/* Receiver is idle */ +#define USR1_AIRINT (1<<5)/* Aysnch IR interrupt */ +#define USR1_AWAKE (1<<4)/* Falling edge detected on RXd pin */ + +#define USR2_ADET (1<<15) /* Autobaud complete */ +#define USR2_TXFE (1<<14) /* Transmit FIFO empty */ +#define USR2_DTRF (1<<13) /* DTR/DSR transition */ +#define USR2_IDLE (1<<12) /* UART has been idle for too long */ +#define USR2_ACST (1<<11) /* Autobaud counter stopped */ +#define USR2_RIDELT (1<<10) /* Ring Indicator delta */ +#define USR2_RIIN (1<<9)/* Ring Indicator Input */ +#define USR2_IRINT (1<<8)/* Serial Infrared Interrupt */ +#define USR2_WAKE (1<<7)/* Start bit detected */ +#define USR2_DCDDELT(1<<6)/* Data Carrier Detect delta */ +#define USR2_DCDIN (1<<5)/* Data Carrier Detect Input */ +#define USR2_RTSF (1<<4)/* RTS transition */ +#define USR2_TXDC (1<<3)/* Transmission complete */ +#define USR2_BRCD (1<<2)/* Break condition detected */ +#define USR2_ORE
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
On Mon, Apr 30, 2012 at 4:52 PM, Michael Tokarev wrote: > This value is used currently for virtio-blk only. It was defined > as uint16_t before, which is the same as in kernel<=>user interface > (in virtio_blk.h, struct virtio_blk_config). But the problem is > that in kernel<=>user interface the units are sectors (which is > usually 512 bytes or more), while in qemu it is in bytes. However, > for, say, md raid5 arrays, it is typical to have actual min_io_size > of a host device to be large. For example, for raid5 device of > 3 drives with 64Kb chunk size, that value will be 128Kb, which does > not fit in a uint16_t anymore. > > Increase the value size from 16bits to 32bits for now. > > But apparently, the kernel<=>user interface needs to be fixed too > (while it is much more difficult due to compatibility issues), > because even with 512byte units, the 16bit value there will overflow > too with quite normal MD RAID configuration. > > Signed-off-by: Michael Tokarev > --- > block.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Which kernel<=>user interface? Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] Poking a sun4v machine
Am 30.04.2012 19:38, schrieb Artyom Tarasenko: > On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber wrote: >> Am 30.04.2012 18:39, schrieb Artyom Tarasenko: >>> Tried to boot QEMU Niagara machine with the firmware from the >>> OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) >>> , and it dies very early. >>> The reason: in translate.c >>> >>> #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) >>> #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) >>> >>> and the dc->mem_idx is initialized like this: >>> >>> if (env1->tl > 0) { >>> return MMU_NUCLEUS_IDX; >>> } else if (cpu_hypervisor_mode(env1)) { >>> return MMU_HYPV_IDX; >>> } else if (cpu_supervisor_mode(env1)) { >>> return MMU_KERNEL_IDX; >>> } else { >>> return MMU_USER_IDX; >>> } >>> >>> Which seems to be conceptually incorrect. After reset tl == MAXTL, but >>> still super- and hyper-visor bits are set, so both supervisor(dc) and >>> hypervisor(dc) must return 1 which is impossible in the current >>> implementation. >>> >>> What would be the proper way to fix it? Make mem_idx bitmap, add two >>> more variables to DisasContext, or ...? >>> >>> Some other findings/questions: >>> >>> /* Sun4v generic Niagara machine */ >>> { >>> .default_cpu_model = "Sun UltraSparc T1", >>> .console_serial_base = 0xfff0c2c000ULL, >>> >>> Where is this address coming from? The OpenSPARC Niagara machine has a >>> "dumb serial" at 0x1f1000ULL. >>> >>> And the biggest issue: UA2005 (as well as UA2007) describe a totally >>> different format for a MMU TTE entry than the one sun4u CPU are using. >>> I think the best way to handle it would be splitting off Niagara >>> machine, and #defining MMU bits differently for sun4u and sun4v >>> machines. >>> >>> Do we the cases in qemu where more than two (qemu-system-xxx and >>> qemu-system-xxx64) binaries are produced? >>> Would the name qemu-system-sun4v fit the naming convention? >> >> We have such a case for ppc (ppcemb) and it is kind of a maintenance >> nightmare - I'm working towards getting rid of it with my QOM CPU work. >> Better avoid it for sparc in the first place. >> >> Instead, you should add a callback function pointer to SPARCCPUClass >> that you initialize based on CPU model so that is behaves differently at >> runtime rather than at compile time. >> Or if it's just about the class_init then after the Hard Freeze I can >> start polishing my subclasses for sparc so that you can add a special >> class_init for Niagara. > > But this would mean that the defines from > #define TTE_NFO_BIT (1ULL << 60) > to > #define TTE_PGSIZE(tte) (((tte) >> 61) & 3ULL) > > inclusive would need to be replaced with functions and variables? > Sounds like a further performance regression for sun4u? First of all, I wouldn't use the term performance regression for a target such as sparc64 that has never really worked before. ;-) I don't know the details of the SPARC implementation or architecture. What I am telling you is that there are ways to implement sun4u and sun4v inside one executable; whether you use a new function or extend a macro with an additional parameter is up to you and Blue. My guess is that unlike ARM you won't need to mix different cores at runtime for the time being. In ARM we have a macro IS_M() that distinguishes between the Cortex-M and Cortex-A families rather than moving Cortex-M to its own executable. The Freescale Vybrid MCUs are going to combine the two in one SoC, i.e. in one qemu-system-arm executable simultaneously. > And would it be possible to have a different register set for an > inherited SPARCCPUClass ? You can add new state fields to an inherited SPARCCPU. If you need them in TCG as offsets from env, then CPUSPARCState might be an easier place to add them but there you cannot distinguish between the models. To an inherited SPARCCPUClass you can add fields for saving constant, e.g., reset values for registers of the model. The unanswered question is whether SPARC should follow the x86 model with declarative (and user-specified) definitions (in that case extending SPARCCPUClass) or the ARM model with imperative per-model init functions (in that case probably not extending SPARCCPU at all). 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] rbd: add discard support
On Tue, May 1, 2012 at 7:16 AM, Josh Durgin wrote: > Change the write flag to an operation type in RBDAIOCB, and make the > buffer optional since discard doesn't use it. > > Discard is first included in librbd 0.1.2 (which is in Ceph 0.46). > If librbd is too old, leave out qemu_rbd_aio_discard entirely, > so the old behavior is preserved. > > Signed-off-by: Josh Durgin > --- > block/rbd.c | 89 -- > 1 files changed, 73 insertions(+), 16 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
On 01.05.2012 12:27, Stefan Hajnoczi wrote: > On Mon, Apr 30, 2012 at 4:52 PM, Michael Tokarev wrote: >> This value is used currently for virtio-blk only. It was defined >> as uint16_t before, which is the same as in kernel<=>user interface >> (in virtio_blk.h, struct virtio_blk_config). But the problem is >> that in kernel<=>user interface the units are sectors (which is >> usually 512 bytes or more), while in qemu it is in bytes. However, >> for, say, md raid5 arrays, it is typical to have actual min_io_size >> of a host device to be large. For example, for raid5 device of >> 3 drives with 64Kb chunk size, that value will be 128Kb, which does >> not fit in a uint16_t anymore. >> >> Increase the value size from 16bits to 32bits for now. >> >> But apparently, the kernel<=>user interface needs to be fixed too >> (while it is much more difficult due to compatibility issues), >> because even with 512byte units, the 16bit value there will overflow >> too with quite normal MD RAID configuration. >> >> Signed-off-by: Michael Tokarev >> --- >> block.h |4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > Which kernel<=>user interface? struct virtio_blk_config in virtio_blk.h, which is used to pass information about block device from (qemu) userspace to guest kernel. Besides, it appears that at least minimum_io_size is not used anywhere in the kernel, -- so, for example, filesystems does not directly benefit from seeing this information. But mkfs.xfs do use it. > Reviewed-by: Stefan Hajnoczi Thanks, /mjt
[Qemu-devel] [PATCH 1/8] booke:Use MMU API for creating initial mapping for secondary cpus
From: Bharat Bhushan Initial Mapping creation for secondary CPU in SMP was missing new MMU API. Signed-off-by: Bharat Bhushan Signed-off-by: Alexander Graf --- hw/ppce500_spin.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c index 960b7b0..95a2825 100644 --- a/hw/ppce500_spin.c +++ b/hw/ppce500_spin.c @@ -86,6 +86,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env, tlb->mas2 = (va & TARGET_PAGE_MASK) | MAS2_M; tlb->mas7_3 = pa & TARGET_PAGE_MASK; tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX; +env->tlb_dirty = true; } static void spin_kick(void *data) -- 1.6.0.2
Re: [Qemu-devel] [PATCH for-1.1 0/2] qemu-ga: Build fixes for Solaris/illumos
On Mon, Apr 30, 2012 at 5:00 PM, Andreas Färber wrote: > Hello, > > Here's a mini-series fixing the build on illumos like I had requested Lee to > do. > > Patch 1 introduces a Solaris-only workaround for O_ASYNC that we could > generalize for 1.2 (ifdef O_ASYNC seemed a bit risky to me on Hard Freeze eve > in case some platform uses an enum rather than preprocessor define, given that > ioctl() is consider "obsolescent" and I_SETSIG optional in POSIX). > > Patch 2 adds libraries needed to link qemu-ga on Solaris, reusing the existing > set of backwards-compatible network libraries rather than the new libxnet. > > Regards, > Andreas > > Cc: Michael Roth > Cc: Lee Essen > Cc: Stefan Hajnoczi > Cc: Blue Swirl > > Andreas Färber (2): > qemu-ga: Implement alternative to O_ASYNC > configure: Add libraries for qemu-ga on Solaris > > configure | 4 +++- > qga/channel-posix.c | 18 +- > 2 files changed, 20 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH 1/2] async: Use bool for boolean struct members and remove a hole
On Sun, Apr 29, 2012 at 07:08:45PM +0200, Stefan Weil wrote: > Using bool reduces the size of the structure and improves readability. > A hole in the structure was removed. > > Signed-off-by: Stefan Weil > --- > async.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) Thanks, applied both to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode
On Mon, Apr 30, 2012 at 11:51, Alexander Graf wrote: > > On 30.04.2012, at 12:45, Alexander Graf wrote: > >> >> On 22.04.2012, at 15:26, Blue Swirl wrote: >> >>> Add an explicit CPUPPCState parameter instead of relying on AREG0 >>> and rename op_helper.c (which only contains load and store helpers) >>> to mem_helper.c. Remove AREG0 swapping in >>> tlb_fill(). >>> >>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation >>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores. >> >> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm >> trying to debug it down, but worst case I'll omit this patch set for 1.1. > > Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. > It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path > is missing completely. > > This actually makes me less confident that this is a change we want for 1.1. > I'll remove the patches from the queue. Meh. It should be perfectly OK to apply all patches except the last one which enables the AREG0 free mode. Also the problem with last patch is not in the patch itself but PPC TCG host support, which by the way is probably also broken for AREG0 free Sparc and Alpha, so I'd really like to see them in 1.1. There should be plenty of time to fix bugs in PPC TCG support during the freeze. > > > Alex > > > TCG register swizzling code: > > #ifdef CONFIG_TCG_PASS_AREG0 > /* XXX/FIXME: suboptimal */ > tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3], > tcg_target_call_iarg_regs[2]); > tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], > tcg_target_call_iarg_regs[1]); > tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1], > tcg_target_call_iarg_regs[0]); > tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], > TCG_AREG0); > #endif > tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1); > > Log output: > > NIP fff024e4 LR CTR XER > > MSR HID0 6000 HF idx 1 > TB 01083771 DECR 4293883502 > GPR00 fff0 > GPR04 24b0 > GPR08 > GPR12 > GPR16 > GPR20 > GPR24 > GPR28 > CR 8000 [ L - - - - - - - ] RES > FPR00 > FPR04 > FPR08 > FPR12 > FPR16 > FPR20 > FPR24 > FPR28 > FPSCR > SRR0 SRR1 PVR 003c0301 VRSAVE > > SPRG0 SPRG1 SPRG2 SPRG3 > > SPRG4 SPRG5 SPRG6 SPRG7 > > SDR1 > IN: > 0xfff024e4: stw r6,0(r4) > > OP: > 0xfff024e4 > movi_i32 access_type,$0x20 > mov_i32 tmp0,r4_0 > movi_i32 tmp1,$0x0 > qemu_st32 r6_0,tmp0,tmp1,$0x1 > goto_tb $0x0 > movi_i32 nip_0,$0xfff024e8 > movi_i32 nip_1,$0x0 > exit_tb $0xf4bae508 > > OUT: [size=180] > 0xf5faf7a0: lwz r14,36(r27) > 0xf5faf7a4: lwz r15,52(r27) > 0xf5faf7a8: li r16,0 > 0xf5faf7ac: li r17,32 > 0xf5faf7b0: stw r17,672(r27) > 0xf5faf7b4: rlwinm r3,r14,25,19,26 > 0xf5faf7b8: add r3,r3,r27 > 0xf5faf7bc: lwzu r4,8912(r3) > 0xf5faf7c0: rlwinm r0,r14,0,30,19 > 0xf5faf7c4: cmpw cr7,r0,r4 > 0xf5faf7c8: lwz r4,4(r3) > 0xf5faf7cc: cmpw cr6,r16,r4 > 0xf5faf7d0: crand 4*cr7+eq,4*cr6+eq,4*cr7+eq > 0xf5faf7d4: beq- cr7,0xf5faf80c > 0xf5faf7d8: mr r3,r16 > 0xf5faf7dc: mr r4,r14 > 0xf5faf7e0: mr r5,r15 > 0xf5faf7e4: li r6,1 > 0xf5faf7e8: mr r6,r5 > 0xf5faf7ec: mr r5,r4 > 0xf5faf7f0: mr r4,r3 > 0xf5faf7f4: mr r3,r27 > 0xf5faf7f8: lis r0,4123 > 0xf5faf7fc: ori r0,r0,27696 > 0xf5faf800: mtctr r0 > 0xf5faf804: bctrl > 0xf5faf808: b 0xf5faf818 >
Re: [Qemu-devel] Poking a sun4v machine
On Mon, Apr 30, 2012 at 16:39, Artyom Tarasenko wrote: > Tried to boot QEMU Niagara machine with the firmware from the > OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) > , and it dies very early. > The reason: in translate.c > > #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) > #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) > > and the dc->mem_idx is initialized like this: > > if (env1->tl > 0) { > return MMU_NUCLEUS_IDX; > } else if (cpu_hypervisor_mode(env1)) { > return MMU_HYPV_IDX; > } else if (cpu_supervisor_mode(env1)) { > return MMU_KERNEL_IDX; > } else { > return MMU_USER_IDX; > } > > Which seems to be conceptually incorrect. After reset tl == MAXTL, but > still super- and hyper-visor bits are set, so both supervisor(dc) and > hypervisor(dc) must return 1 which is impossible in the current > implementation. I don't think this is needed. The MMU index tells which TLB is used for guest virtual to host address translations, during tl == MAXTL we want to use hypervisor mode translations. > > What would be the proper way to fix it? Make mem_idx bitmap, add two > more variables to DisasContext, or ...? > > Some other findings/questions: > > /* Sun4v generic Niagara machine */ > { > .default_cpu_model = "Sun UltraSparc T1", > .console_serial_base = 0xfff0c2c000ULL, > > Where is this address coming from? The OpenSPARC Niagara machine has a > "dumb serial" at 0x1f1000ULL. I think I actually used Ontario machine definitions. > > And the biggest issue: UA2005 (as well as UA2007) describe a totally > different format for a MMU TTE entry than the one sun4u CPU are using. > I think the best way to handle it would be splitting off Niagara > machine, and #defining MMU bits differently for sun4u and sun4v > machines. > > Do we the cases in qemu where more than two (qemu-system-xxx and > qemu-system-xxx64) binaries are produced? > Would the name qemu-system-sun4v fit the naming convention? > > Artyom > > -- > Regards, > Artyom Tarasenko > > solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] Poking a sun4v machine
On Mon, Apr 30, 2012 at 17:38, Artyom Tarasenko wrote: > On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber wrote: >> Am 30.04.2012 18:39, schrieb Artyom Tarasenko: >>> Tried to boot QEMU Niagara machine with the firmware from the >>> OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) >>> , and it dies very early. >>> The reason: in translate.c >>> >>> #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) >>> #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) >>> >>> and the dc->mem_idx is initialized like this: >>> >>> if (env1->tl > 0) { >>> return MMU_NUCLEUS_IDX; >>> } else if (cpu_hypervisor_mode(env1)) { >>> return MMU_HYPV_IDX; >>> } else if (cpu_supervisor_mode(env1)) { >>> return MMU_KERNEL_IDX; >>> } else { >>> return MMU_USER_IDX; >>> } >>> >>> Which seems to be conceptually incorrect. After reset tl == MAXTL, but >>> still super- and hyper-visor bits are set, so both supervisor(dc) and >>> hypervisor(dc) must return 1 which is impossible in the current >>> implementation. >>> >>> What would be the proper way to fix it? Make mem_idx bitmap, add two >>> more variables to DisasContext, or ...? >>> >>> Some other findings/questions: >>> >>> /* Sun4v generic Niagara machine */ >>> { >>> .default_cpu_model = "Sun UltraSparc T1", >>> .console_serial_base = 0xfff0c2c000ULL, >>> >>> Where is this address coming from? The OpenSPARC Niagara machine has a >>> "dumb serial" at 0x1f1000ULL. >>> >>> And the biggest issue: UA2005 (as well as UA2007) describe a totally >>> different format for a MMU TTE entry than the one sun4u CPU are using. >>> I think the best way to handle it would be splitting off Niagara >>> machine, and #defining MMU bits differently for sun4u and sun4v >>> machines. >>> >>> Do we the cases in qemu where more than two (qemu-system-xxx and >>> qemu-system-xxx64) binaries are produced? >>> Would the name qemu-system-sun4v fit the naming convention? >> >> We have such a case for ppc (ppcemb) and it is kind of a maintenance >> nightmare - I'm working towards getting rid of it with my QOM CPU work. >> Better avoid it for sparc in the first place. >> >> Instead, you should add a callback function pointer to SPARCCPUClass >> that you initialize based on CPU model so that is behaves differently at >> runtime rather than at compile time. >> Or if it's just about the class_init then after the Hard Freeze I can >> start polishing my subclasses for sparc so that you can add a special >> class_init for Niagara. > > But this would mean that the defines from > #define TTE_NFO_BIT (1ULL << 60) > to > #define TTE_PGSIZE(tte) (((tte) >> 61) & 3ULL) > > inclusive would need to be replaced with functions and variables? > Sounds like a further performance regression for sun4u? There could be parallel definitions for sun4u (actually UltraSparc-III onwards the MMU is again different) and sun4v. At tlb_fill(), different implementations can be selected based on MMU model. For ASI accesses, we can add conditional code but for higher performance, some checks can be moved to translation time. > > And would it be possible to have a different register set for an > inherited SPARCCPUClass ? > Also trap handling and related cpu registers behavior has to be quite > different. Not really, we already handle some (but not all cases). > > Artyom > >> Helpers such as cpu_hypervisor_mode() will need to be changed to take a >> SPARCCPU *cpu rather than CPUSPARCState *env argument; as an interim >> solution sparc_env_get_cpu() can be used. (examples on the list for sh4) >> >> Andreas >> >> -- >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > > > > -- > Regards, > Artyom Tarasenko > > solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
[Qemu-devel] [PULL 0/3] Trivial patches for 21 April to 1 May 2012
The following changes since commit 42fe1c245f0239ebcdc084740a1777ac3699d071: main-loop: Fix build for w32 and w64 (2012-04-28 09:25:54 +) are available in the git repository at: git://github.com/stefanha/qemu.git trivial-patches for you to fetch changes up to c97feed13cded953b11465829f66b9323a47a0f9: iohandler: Use bool for boolean struct member and remove holes (2012-05-01 10:13:33 +0100) Stefan Weil (3): configure: Fix creation of symbolic links for MinGW toolchain async: Use bool for boolean struct members and remove a hole iohandler: Use bool for boolean struct member and remove holes async.c |6 +++--- configure | 21 ++--- iohandler.c |4 ++-- 3 files changed, 15 insertions(+), 16 deletions(-) -- 1.7.10
[Qemu-devel] [PATCH 3/3] iohandler: Use bool for boolean struct member and remove holes
From: Stefan Weil Using bool reduces the size of the structure and improves readability. Two holes in the structure were removed. Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- iohandler.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iohandler.c b/iohandler.c index 5640d49..3c74de6 100644 --- a/iohandler.c +++ b/iohandler.c @@ -33,13 +33,13 @@ #endif typedef struct IOHandlerRecord { -int fd; IOCanReadHandler *fd_read_poll; IOHandler *fd_read; IOHandler *fd_write; -int deleted; void *opaque; QLIST_ENTRY(IOHandlerRecord) next; +int fd; +bool deleted; } IOHandlerRecord; static QLIST_HEAD(, IOHandlerRecord) io_handlers = -- 1.7.10
[Qemu-devel] [PATCH 1/3] configure: Fix creation of symbolic links for MinGW toolchain
From: Stefan Weil The MinGW toolchain on w32/w64 hosts does not create symbolic links, but implements 'ln -s' similar to 'cp -r'. In incremental out of tree builds, this resulted in files which were not updated when their counterparts in the QEMU source tree changed. Especially for Makefile* this happened very often. With this patch, the 'symlinked' files are now always updated for out of tree builds. Similar code was already used for the symbolic link of libcacard/Makefile. The symlink macro always removes the target before it is created again, therefore the rm command for libcacard/Makefile was redundant and is removed now. Macro symlink is also used with directories. To remove them on w32 hosts, a recursive rm is needed. v2: Quote arguments in shell function symlink, and also quote any argument which is passed to symlink and which contains macros. This should reduce the chance of accidents caused by rm -rf. Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- configure | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/configure b/configure index 25697bb..9d21302 100755 --- a/configure +++ b/configure @@ -41,8 +41,8 @@ compile_prog() { # symbolically link $1 to $2. Portable version of "ln -sf". symlink() { - rm -f $2 - ln -s $1 $2 + rm -rf "$2" + ln -s "$1" "$2" } # check whether a command is available to this shell (may be either an @@ -3435,7 +3435,7 @@ fi for d in libdis libdis-user; do mkdir -p $d -symlink $source_path/Makefile.dis $d/Makefile +symlink "$source_path/Makefile.dis" "$d/Makefile" echo > $d/config.mak done @@ -3444,13 +3444,13 @@ if test "$linux" = "yes" ; then mkdir -p linux-headers case "$cpu" in i386|x86_64) -symlink $source_path/linux-headers/asm-x86 linux-headers/asm +symlink "$source_path/linux-headers/asm-x86" linux-headers/asm ;; ppcemb|ppc|ppc64) -symlink $source_path/linux-headers/asm-powerpc linux-headers/asm +symlink "$source_path/linux-headers/asm-powerpc" linux-headers/asm ;; s390x) -symlink $source_path/linux-headers/asm-s390 linux-headers/asm +symlink "$source_path/linux-headers/asm-s390" linux-headers/asm ;; esac fi @@ -3515,7 +3515,7 @@ mkdir -p $target_dir/kvm if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o "$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then mkdir -p $target_dir/nwfpe fi -symlink $source_path/Makefile.target $target_dir/Makefile +symlink "$source_path/Makefile.target" "$target_dir/Makefile" echo "# Automatically generated by configure - do not modify" > $config_target_mak @@ -3958,7 +3958,7 @@ do done mkdir -p $DIRS for f in $FILES ; do -if [ -e "$source_path/$f" ] && ! [ -e "$f" ]; then +if [ -e "$source_path/$f" ] && [ "$source_path" != `pwd` ]; then symlink "$source_path/$f" "$f" fi done @@ -3981,7 +3981,7 @@ for hwlib in 32 64; do mkdir -p $d mkdir -p $d/ide mkdir -p $d/usb - symlink $source_path/Makefile.hw $d/Makefile + symlink "$source_path/Makefile.hw" "$d/Makefile" mkdir -p $d/9pfs echo "QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=$hwlib" > $d/config.mak done @@ -3989,7 +3989,6 @@ done if [ "$source_path" != `pwd` ]; then # out of tree build mkdir -p libcacard -rm -f libcacard/Makefile symlink "$source_path/libcacard/Makefile" libcacard/Makefile fi @@ -3997,7 +3996,7 @@ d=libuser mkdir -p $d mkdir -p $d/trace mkdir -p $d/qom -symlink $source_path/Makefile.user $d/Makefile +symlink "$source_path/Makefile.user" "$d/Makefile" if test "$docs" = "yes" ; then mkdir -p QMP -- 1.7.10
[Qemu-devel] [PATCH 8/8] target-ppc: Some support for dumping TLB_EMB TLBs
From: François Revol Add mmubooke_dump_mmu(). TODO: Add printing of individual flags. Signed-off-by: François Revol [agraf: fix coding style] Signed-off-by: Alexander Graf --- target-ppc/helper.c | 50 ++ 1 files changed, 50 insertions(+), 0 deletions(-) diff --git a/target-ppc/helper.c b/target-ppc/helper.c index c610ce3..e97e496 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -1466,6 +1466,53 @@ static const char *book3e_tsize_to_str[32] = { "1T", "2T" }; +static void mmubooke_dump_mmu(FILE *f, fprintf_function cpu_fprintf, + CPUPPCState *env) +{ +ppcemb_tlb_t *entry; +int i; + +if (kvm_enabled() && !env->kvm_sw_tlb) { +cpu_fprintf(f, "Cannot access KVM TLB\n"); +return; +} + +cpu_fprintf(f, "\nTLB:\n"); +cpu_fprintf(f, "Effective Physical Size PID Prot " +"Attr\n"); + +entry = &env->tlb.tlbe[0]; +for (i = 0; i < env->nb_tlb; i++, entry++) { +target_phys_addr_t ea, pa; +target_ulong mask; +uint64_t size = (uint64_t)entry->size; +char size_buf[20]; + +/* Check valid flag */ +if (!(entry->prot & PAGE_VALID)) { +continue; +} + +mask = ~(entry->size - 1); +ea = entry->EPN & mask; +pa = entry->RPN & mask; +#if (TARGET_PHYS_ADDR_BITS >= 36) +/* Extend the physical address to 36 bits */ +pa |= (target_phys_addr_t)(entry->RPN & 0xF) << 32; +#endif +size /= 1024; +if (size >= 1024) { +snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "M", size / 1024); +} else { +snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "k", size); +} +cpu_fprintf(f, "0x%016" PRIx64 " 0x%016" PRIx64 " %s %-5u %08x %08x\n", +(uint64_t)ea, (uint64_t)pa, size_buf, (uint32_t)entry->PID, +entry->prot, entry->attr); +} + +} + static void mmubooke206_dump_one_tlb(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env, int tlbn, int offset, int tlbsize) @@ -1561,6 +1608,9 @@ static void mmubooks_dump_mmu(FILE *f, fprintf_function cpu_fprintf, void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env) { switch (env->mmu_model) { +case POWERPC_MMU_BOOKE: +mmubooke_dump_mmu(f, cpu_fprintf, env); +break; case POWERPC_MMU_BOOKE206: mmubooke206_dump_mmu(f, cpu_fprintf, env); break; -- 1.6.0.2
[Qemu-devel] [PATCH 4/8] pseries: Implement automatic PAPR VIO address allocation
From: David Gibson PAPR virtual IO (VIO) devices require a unique, but otherwise arbitrary, "address" used as a token to the hypercalls which manipulate them. Currently the pseries machine code does an ok job of allocating these addresses when the legacy -net nic / -serial and so forth options are used but will fail to allocate them properly when using -device. Specifically, you can use -device if all addresses are explicitly assigned. Without explicit assignment, only one VIO device of each type (network, console, SCSI) will be assigned properly, any further ones will attempt to take the same address leading to a fatal error. This patch fixes the situation by adding a proper address allocator to the VIO "bus" code. This is used both by -device and the legacy options and default devices. Addresses can still be explicitly assigned with -device options if desired. This patch changes the (guest visible) numbering of VIO devices, but since their addresses are discovered using the device tree and already differ from the numbering found on existing PowerVM systems, this does not break compatibility. Signed-off-by: David Gibson Signed-off-by: Alexander Graf --- hw/spapr.c |7 +++ hw/spapr_llan.c |5 ++--- hw/spapr_vio.c | 54 ++ hw/spapr_vio.h | 13 ++--- hw/spapr_vscsi.c |5 ++--- hw/spapr_vty.c |5 ++--- 6 files changed, 49 insertions(+), 40 deletions(-) diff --git a/hw/spapr.c b/hw/spapr.c index bfaf260..cca20f9 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -631,8 +631,7 @@ static void ppc_spapr_init(ram_addr_t ram_size, for (i = 0; i < MAX_SERIAL_PORTS; i++) { if (serial_hds[i]) { -spapr_vty_create(spapr->vio_bus, SPAPR_VTY_BASE_ADDRESS + i, - serial_hds[i]); +spapr_vty_create(spapr->vio_bus, serial_hds[i]); } } @@ -650,14 +649,14 @@ static void ppc_spapr_init(ram_addr_t ram_size, } if (strcmp(nd->model, "ibmveth") == 0) { -spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd); +spapr_vlan_create(spapr->vio_bus, nd); } else { pci_nic_init_nofail(&nd_table[i], nd->model, NULL); } } for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) { -spapr_vscsi_create(spapr->vio_bus, 0x2000 + i); +spapr_vscsi_create(spapr->vio_bus); } if (rma_size < (MIN_RMA_SLOF << 20)) { diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c index e18d2eb..8313043 100644 --- a/hw/spapr_llan.c +++ b/hw/spapr_llan.c @@ -204,12 +204,11 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev) return 0; } -void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd) +void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd) { DeviceState *dev; dev = qdev_create(&bus->bus, "spapr-vlan"); -qdev_prop_set_uint32(dev, "reg", reg); qdev_set_nic_properties(dev, nd); @@ -480,7 +479,7 @@ static target_ulong h_multicast_ctrl(CPUPPCState *env, sPAPREnvironment *spapr, } static Property spapr_vlan_properties[] = { -DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x1000), +DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000), DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c index fccf48b..315ab80 100644 --- a/hw/spapr_vio.c +++ b/hw/spapr_vio.c @@ -620,28 +620,22 @@ static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token, rtas_st(rets, 0, 0); } -static int spapr_vio_check_reg(VIOsPAPRDevice *sdev) +static VIOsPAPRDevice *reg_conflict(VIOsPAPRDevice *dev) { -VIOsPAPRDevice *other_sdev; +VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus); DeviceState *qdev; -VIOsPAPRBus *sbus; - -sbus = DO_UPCAST(VIOsPAPRBus, bus, sdev->qdev.parent_bus); +VIOsPAPRDevice *other; /* - * Check two device aren't given clashing addresses by the user (or some - * other mechanism). We have to open code this because we have to check - * for matches with devices other than us. + * Check for a device other than the given one which is already + * using the requested address. We have to open code this because + * the given dev might already be in the list. */ -QTAILQ_FOREACH(qdev, &sbus->bus.children, sibling) { -other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, qdev); +QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) { +other = DO_UPCAST(VIOsPAPRDevice, qdev, qdev); -if (other_sdev != sdev && other_sdev->reg == sdev->reg) { -fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n", -object_get_typename(OBJECT(sdev)), -object_get_typename(OBJECT(qdev)), -sdev->reg); -return -EEXIST; +if (other != dev && other->reg == d
[Qemu-devel] [PATCH 2/3] async: Use bool for boolean struct members and remove a hole
From: Stefan Weil Using bool reduces the size of the structure and improves readability. A hole in the structure was removed. Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- async.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/async.c b/async.c index ecdaf15..85cc641 100644 --- a/async.c +++ b/async.c @@ -35,10 +35,10 @@ static struct QEMUBH *first_bh; struct QEMUBH { QEMUBHFunc *cb; void *opaque; -int scheduled; -int idle; -int deleted; QEMUBH *next; +bool scheduled; +bool idle; +bool deleted; }; QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) -- 1.7.10
[Qemu-devel] [PATCH 7/8] ppce500_spin: Replace assert by hw_error (fixes compiler warning)
From: Stefan Weil The default case in function spin_read should never be reached, therefore the old code used assert(0) to abort QEMU. This does not work when QEMU is compiled with macro NDEBUG defined. In this case (and also when the compiler does not know that assert never returns), there is a compiler warning because of the missing return value. Using hw_error allows an improved error message and aborts always. Signed-off-by: Stefan Weil [agraf: use __func__] Signed-off-by: Alexander Graf --- hw/ppce500_spin.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c index 95a2825..fddf219 100644 --- a/hw/ppce500_spin.c +++ b/hw/ppce500_spin.c @@ -179,7 +179,7 @@ static uint64_t spin_read(void *opaque, target_phys_addr_t addr, unsigned len) case 4: return ldl_p(spin_p); default: -assert(0); +hw_error("ppce500: unexpected %s with len = %u", __func__, len); } } -- 1.6.0.2
[Qemu-devel] [PATCH 5/8] pseries: Use the same interrupt swizzling for host bridges as p2p bridges
From: David Gibson Currently the pseries PCI code uses a somewhat strange scheme of PCI irq allocation - one per slot up to a maximum that's greater than the usual 4. This scheme more or less worked, because we were able to tell the guest the irq mapping in the device tree, however it's a bit odd and may break assumptions in the future. Worse, the array used to construct the dev tree interrupt map was mis-sized, we got away with it only because it happened that our SPAPR_PCI_NUM_LSI value was greater than 7. This patch changes the pseries PCI code to use the same interrupt swizzling scheme as is standardized for PCI to PCI bridges. This makes for better consistency, deals better with any devices which use multiple interrupt pins and will make life easier in the future when we add passthrough of what may be either a host bridge or a PCI to PCI bridge. This won't break existing guests, because they don't assume a particular mapping scheme for host bridges, but just follow what we tell them in the device tree (also updated to match, of course). This patch also fixes the allocation of the irq map. Signed-off-by: David Gibson Signed-off-by: Alexander Graf --- hw/spapr_pci.c | 49 - hw/spapr_pci.h |5 ++--- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c index a564c00..25b400a 100644 --- a/hw/spapr_pci.c +++ b/hw/spapr_pci.c @@ -198,16 +198,20 @@ static void rtas_write_pci_config(sPAPREnvironment *spapr, finish_write_pci_config(spapr, 0, addr, size, val, rets); } +static int pci_spapr_swizzle(int slot, int pin) +{ +return (slot + pin) % PCI_NUM_PINS; +} + static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num) { /* * Here we need to convert pci_dev + irq_num to some unique value - * which is less than number of IRQs on the specific bus (now it - * is 16). At the moment irq_num == device_id (number of the - * slot?) - * FIXME: we should swizzle in fn and irq_num + * which is less than number of IRQs on the specific bus (4). We + * use standard PCI swizzling, that is (slot number + pin number) + * % 4. */ -return (pci_dev->devfn >> 3) % SPAPR_PCI_NUM_LSI; +return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num); } static void pci_spapr_set_irq(void *opaque, int irq_num, int level) @@ -304,13 +308,13 @@ static int spapr_phb_init(SysBusDevice *s) phb->busname ? phb->busname : phb->dtbusname, pci_spapr_set_irq, pci_spapr_map_irq, phb, &phb->memspace, &phb->iospace, - PCI_DEVFN(0, 0), SPAPR_PCI_NUM_LSI); + PCI_DEVFN(0, 0), PCI_NUM_PINS); phb->host_state.bus = bus; QLIST_INSERT_HEAD(&spapr->phbs, phb, list); /* Initialize the LSI table */ -for (i = 0; i < SPAPR_PCI_NUM_LSI; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { qemu_irq qirq; uint32_t num; @@ -392,8 +396,7 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt) { -PCIBus *bus = phb->host_state.bus; -int bus_off, i; +int bus_off, i, j; char nodename[256]; uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) }; struct { @@ -415,8 +418,8 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb, }; uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 }; uint32_t interrupt_map_mask[] = { -cpu_to_be32(b_d(-1)|b_fff(0)), 0x0, 0x0, 0x0}; -uint32_t interrupt_map[bus->nirq][7]; +cpu_to_be32(b_d(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; +uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; /* Start populating the FDT */ sprintf(nodename, "pci@%" PRIx64, phb->buid); @@ -450,19 +453,23 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb, */ _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask", &interrupt_map_mask, sizeof(interrupt_map_mask))); -for (i = 0; i < 7; i++) { -uint32_t *irqmap = interrupt_map[i]; -irqmap[0] = cpu_to_be32(b_d(i)|b_fff(0)); -irqmap[1] = 0; -irqmap[2] = 0; -irqmap[3] = 0; -irqmap[4] = cpu_to_be32(xics_phandle); -irqmap[5] = cpu_to_be32(phb->lsi_table[i % SPAPR_PCI_NUM_LSI].dt_irq); -irqmap[6] = cpu_to_be32(0x8); +for (i = 0; i < PCI_SLOT_MAX; i++) { +for (j = 0; j < PCI_NUM_PINS; j++) { +uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j]; +int lsi_num = pci_spapr_swizzle(i, j); + +irqmap[0] = cpu_to_be32(b_d(i)|b_fff(0)); +irqmap[1] = 0; +irqmap[2] = 0; +irqmap[3] = cpu_to_be32(j+1); +irqmap[4] = cpu_to_be32(xics_phandle); +irqmap[5] = cpu_to_be32(phb->lsi_table[lsi_num].dt_irq); +
[Qemu-devel] [PATCH 6/8] pseries: Fix use of global CPU state
From: Peter Portante Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA functions for pSeries shared processor partitions) introduced the deregister_dtl() function and typo "emv" as name of its argument. This went unnoticed because the code in that function can access the global variable "env" so that no build failure resulted. Fix the argument to read "env". Resolves LP#986241. Signed-off-by: Peter Portante Acked-by: Andreas Färber [agraf: fixed typo in commit message] Signed-off-by: Alexander Graf --- hw/spapr_hcall.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c index 634763e..94bb504 100644 --- a/hw/spapr_hcall.c +++ b/hw/spapr_hcall.c @@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env, target_ulong addr) return H_SUCCESS; } -static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr) +static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr) { env->dispatch_trace_log = 0; env->dtl_size = 0; -- 1.6.0.2
[Qemu-devel] [PULL 0/8] ppc patch queue 2012-05-01
Hi Anthony, This is my current patch queue for ppc. Please pull. Alex The following changes since commit 42fe1c245f0239ebcdc084740a1777ac3699d071: Stefan Weil (1): main-loop: Fix build for w32 and w64 are available in the git repository at: git://repo.or.cz/qemu/agraf.git ppc-for-upstream Alexander Graf (2): PPC: Fix up e500 cache size setting linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts Bharat Bhushan (1): booke:Use MMU API for creating initial mapping for secondary cpus David Gibson (2): pseries: Implement automatic PAPR VIO address allocation pseries: Use the same interrupt swizzling for host bridges as p2p bridges François Revol (1): target-ppc: Some support for dumping TLB_EMB TLBs Peter Portante (1): pseries: Fix use of global CPU state Stefan Weil (1): ppce500_spin: Replace assert by hw_error (fixes compiler warning) hw/ppce500_spin.c |3 +- hw/spapr.c |7 ++--- hw/spapr_hcall.c|2 +- hw/spapr_llan.c |5 +-- hw/spapr_pci.c | 49 ++ hw/spapr_pci.h |5 +-- hw/spapr_vio.c | 54 +++ hw/spapr_vio.h | 13 - hw/spapr_vscsi.c|5 +-- hw/spapr_vty.c |5 +-- target-ppc/helper.c | 50 +++ target-ppc/translate_init.c | 26 +++- thunk.h |2 +- 13 files changed, 147 insertions(+), 79 deletions(-)
[Qemu-devel] [PATCH 2/8] PPC: Fix up e500 cache size setting
When initializing the e500 code, we need to expose its cache line size for user and system mode, while the mmu details are only interesting for system emulation. Split the 2 switch statements apart, allowing us to #ifdef out the mmu parts for user mode emulation while keeping all cache information consistent. Signed-off-by: Alexander Graf --- target-ppc/translate_init.c | 26 ++ 1 files changed, 14 insertions(+), 12 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index ba4b84d..6f61175 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -4461,33 +4461,36 @@ static void init_proc_e500 (CPUPPCState *env, int version) &spr_read_spefscr, &spr_write_spefscr, &spr_read_spefscr, &spr_write_spefscr, 0x); +#if !defined(CONFIG_USER_ONLY) /* Memory management */ -#if defined(CONFIG_USER_ONLY) -env->dcache_line_size = 32; -env->icache_line_size = 32; -#else /* !defined(CONFIG_USER_ONLY) */ env->nb_pids = 3; env->nb_ways = 2; env->id_tlbs = 0; switch (version) { case fsl_e500v1: -/* e500v1 */ tlbncfg[0] = gen_tlbncfg(2, 1, 1, 0, 256); tlbncfg[1] = gen_tlbncfg(16, 1, 9, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); -env->dcache_line_size = 32; -env->icache_line_size = 32; break; case fsl_e500v2: -/* e500v2 */ tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16); -env->dcache_line_size = 32; -env->icache_line_size = 32; break; case fsl_e500mc: -/* e500mc */ tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512); tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64); +break; +default: +cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]); +} +#endif +/* Cache sizes */ +switch (version) { +case fsl_e500v1: +case fsl_e500v2: +env->dcache_line_size = 32; +env->icache_line_size = 32; +break; +case fsl_e500mc: env->dcache_line_size = 64; env->icache_line_size = 64; l1cfg0 |= 0x100; /* 64 byte cache block size */ @@ -4495,7 +4498,6 @@ static void init_proc_e500 (CPUPPCState *env, int version) default: cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]); } -#endif gen_spr_BookE206(env, 0x00DF, tlbncfg); /* XXX : not implemented */ spr_register(env, SPR_HID0, "HID0", -- 1.6.0.2
[Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts
On my PPC host, HOST_LONG_SIZE is not defined even after running configure. Use the normal C way of determining the long size instead. Signed-off-by: Alexander Graf --- thunk.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/thunk.h b/thunk.h index 5be8f91..87025c3 100644 --- a/thunk.h +++ b/thunk.h @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype *type_ptr, int is_host) defined(HOST_PARISC) || defined(HOST_SPARC64) return 4; #elif defined(HOST_PPC) -return HOST_LONG_SIZE; +return sizeof(void *); #else return 2; #endif -- 1.6.0.2
Re: [Qemu-devel] [PATCH 1/4] exec: prepare for splitting
Am 22.04.2012 17:35, schrieb Blue Swirl: > Make s_cputlb_empty_entry 'const'. > > Rename tlb_flush_jmp_cache() to tb_flush_jmp_cache(). > > Refactor code to add cpu_tlb_reset_dirty_all(), > memory_region_section_get_iotlb() and > memory_region_is_unassigned(). > > Remove unused cpu_tlb_update_dirty(). > > Fix coding style in areas to be moved. > > Signed-off-by: Blue Swirl This patch is line-wrapped. It's even worse on Patchwork. I fixed the mail up and applied it manually with fuzz (the exec.c 46xx hunk). Compiles fine on openSUSE 12.1 x86_64, and so do mingw32 and mingw64 cross builds. Tested-by: Andreas Färber 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 v2] sdl: Add QEMU mascot icon for use with SDL
Am 23.04.2012 22:11, schrieb Stefan Weil: > Am 13.04.2012 22:24, schrieb Stefan Weil: >> This is a bitmap file (32x32x4) derived from the official QEMU mascot >> (which was designed by Benoît Canet). I stripped the text from the SVG >> to get a nearly square image and converted the result to BMP without >> any manual optimization. >> >> The bitmap is currently used by QEMU's SDL interface and replaces the >> default X icon. >> >> v2: Add qemu-icon.bmp to Makefile. >> >> Signed-off-by: Stefan Weil >> --- >> Makefile | 1 + >> pc-bios/qemu-icon.bmp | Bin 0 -> 630 bytes >> 2 files changed, 1 insertions(+), 0 deletions(-) >> create mode 100644 pc-bios/qemu-icon.bmp [...] > I'm a bit surprised that there was no reaction on this patch > (neither on version 1 nor on version 2). It looks kind of ugly in GNOME3, where it is scaled up to some other resolution in the top bar. IIRC Windows 7 uses 64x64 max, Mac OS X 128x128 max, GNOME 48x48(?). When we add icons for Gtk+ and Cocoa we might also want to rename the file to indicate size and/or purpose (e.g., qemu-sdl-icon.bmp / qemu-32x32-icon.bmp). For Windows, wouldn't it be a better idea to use the multi-resolution .ico format through version.rc and to avoid the generic SDL icon being used? Don't see that being set up in this patch - #ifdef it out there? 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] [PULL] QOM CPUState for sh4, m68k and mips
On Wed, Apr 25, 2012 at 14:23, Andreas Färber wrote: > Hello Anthony, Blue, > > Please pull the remainder of the QOM CPU conversions: sh4, m68k and mips. > > There was no reaction from the listed maintainers to my patches for weeks > nor to the MAINTAINERS RFC for one week, so please apply to let us proceed. Thanks, pulled. > > Cc: Anthony Liguori > Cc: Blue Swirl > > Cc: Aurélien Jarno > Cc: Paul Brook > > The following changes since commit cf36b31db209a261ee3bc2737e788e1ced0a1bec: > > Limit ptimer rate to something achievable (2012-04-24 09:50:31 -0500) > > are available in the git repository at: > git://github.com/afaerber/qemu-cpu.git qom-cpu-rest.v1 > > Andreas Färber (12): > MAINTAINERS: Downgrade target-m68k to Odd Fixes > MAINTAINERS: Downgrade target-mips and target-sh4 to Odd Fixes > target-sh4: QOM'ify CPU > target-sh4: QOM'ify CPU reset > target-sh4: Start QOM'ifying CPU init > target-m68k: QOM'ify CPU > target-m68k: QOM'ify CPU reset > target-m68k: Start QOM'ifying CPU init > target-m68k: Add QOM CPU subclasses > target-mips: QOM'ify CPU > target-mips: Start QOM'ifying CPU init > Makefile: Simplify compilation of target-*/cpu.c > > MAINTAINERS | 6 +- > Makefile.target | 11 +--- > target-m68k/cpu-qom.h | 70 +++ > target-m68k/cpu.c | 170 > +++ > target-m68k/cpu.h | 3 +- > target-m68k/helper.c | 159 ++-- > target-mips/cpu-qom.h | 74 > target-mips/cpu.c | 69 +++ > target-mips/cpu.h | 2 + > target-mips/translate.c | 5 +- > target-sh4/cpu-qom.h | 70 +++ > target-sh4/cpu.c | 90 + > target-sh4/cpu.h | 2 + > target-sh4/translate.c | 28 ++-- > 14 files changed, 612 insertions(+), 147 deletions(-) > create mode 100644 target-m68k/cpu-qom.h > create mode 100644 target-m68k/cpu.c > create mode 100644 target-mips/cpu-qom.h > create mode 100644 target-mips/cpu.c > create mode 100644 target-sh4/cpu-qom.h > create mode 100644 target-sh4/cpu.c
Re: [Qemu-devel] [PULL v2] PReP patch queue 2012-04-30
On Mon, Apr 30, 2012 at 15:24, Andreas Färber wrote: > Hello Blue, > > Please pull the PowerPC Reference Platform (PReP) queue into qemu.git master. Thanks, pulled. > > The following changes since commit 42fe1c245f0239ebcdc084740a1777ac3699d071: > > main-loop: Fix build for w32 and w64 (2012-04-28 09:25:54 +) > > are available in the git repository at: > git://repo.or.cz/qemu/afaerber.git prep-up > > Hervé Poussineau (5): > i82378/i82374: Do not create DMA controller twice > fdc: Parametrize ISA base, IRQ and DMA > isa: Add isa_bus_from_device() method > prep: Initialize PC speaker > prep: Move int-ack register from PReP to Raven PCI emulation > > hw/fdc.c | 17 ++--- > hw/i82374.c | 5 - > hw/i82378.c | 5 +++-- > hw/isa.h | 5 + > hw/ppc_prep.c | 40 > hw/prep_pci.c | 17 + > 6 files changed, 43 insertions(+), 46 deletions(-)
Re: [Qemu-devel] [PULL] Cocoa patch queue 2012-05-01
On Mon, Apr 30, 2012 at 22:15, Andreas Färber wrote: > Hello Blue, > > Please pull the Cocoa queue into qemu.git master. Thanks, pulled. > > The following changes since commit 42fe1c245f0239ebcdc084740a1777ac3699d071: > Stefan Weil (1): > main-loop: Fix build for w32 and w64 > > are available in the git repository at: > > git://repo.or.cz/qemu/afaerber.git cocoa-for-upstream > > Andreas Färber (1): > Drop darwin-user > > Pavel Borzenkov (2): > raw-posix: Do not use CONFIG_COCOA macro > configure: add '--disable-cocoa' switch > > MAINTAINERS | 5 - > Makefile.target | 28 - > block/raw-posix.c | 8 +- > configure | 30 +- > darwin-user/commpage.c | 357 > darwin-user/ioctls.h | 4 - > darwin-user/ioctls_types.h | 1 - > darwin-user/machload.c | 902 --- > darwin-user/main.c | 1027 -- > darwin-user/mmap.c | 409 - > darwin-user/qemu.h | 178 > darwin-user/signal.c | 452 -- > darwin-user/syscall.c | 1566 > -- > darwin-user/syscalls.h | 384 - > default-configs/i386-darwin-user.mak | 1 - > default-configs/ppc-darwin-user.mak | 3 - > qemu-doc.texi | 90 -- > qemu-tech.texi | 3 +- > 18 files changed, 9 insertions(+), 5439 deletions(-) > delete mode 100644 darwin-user/commpage.c > delete mode 100644 darwin-user/ioctls.h > delete mode 100644 darwin-user/ioctls_types.h > delete mode 100644 darwin-user/machload.c > delete mode 100644 darwin-user/main.c > delete mode 100644 darwin-user/mmap.c > delete mode 100644 darwin-user/qemu.h > delete mode 100644 darwin-user/signal.c > delete mode 100644 darwin-user/syscall.c > delete mode 100644 darwin-user/syscalls.h > delete mode 100644 default-configs/i386-darwin-user.mak > delete mode 100644 default-configs/ppc-darwin-user.mak
Re: [Qemu-devel] [PULL] MAINTAINERS tidying and stable trees
On Wed, Apr 25, 2012 at 15:17, Andreas Färber wrote: > Hello Anthony, > > Please pull the uncontroversial parts of MAINTAINERS updates. > > I'm leaving out the Maintained upgrades and would ask you to apply the 0.15 > patch once we've figured that out. For darwin-user I'll send a separate PULL. Thanks, pulled. > > Cc: Anthony Liguori > > The following changes since commit cf36b31db209a261ee3bc2737e788e1ced0a1bec: > > Limit ptimer rate to something achievable (2012-04-24 09:50:31 -0500) > > are available in the git repository at: > git://repo.or.cz/qemu/afaerber.git maintainers-up > > Andreas Färber (6): > MAINTAINERS: Fix PC file pattern > MAINTAINERS: Fix virtio-9p file pattern > MAINTAINERS: Fix TCI file pattern > MAINTAINERS: Indicate type of SCM > MAINTAINERS: Fix SCM tree for virtio-9p > MAINTAINERS: Document all stable trees > > MAINTAINERS | 41 - > 1 files changed, 32 insertions(+), 9 deletions(-) >
Re: [Qemu-devel] [PATCH 1/4] exec: prepare for splitting
On Tue, May 1, 2012 at 11:02, Andreas Färber wrote: > Am 22.04.2012 17:35, schrieb Blue Swirl: >> Make s_cputlb_empty_entry 'const'. >> >> Rename tlb_flush_jmp_cache() to tb_flush_jmp_cache(). >> >> Refactor code to add cpu_tlb_reset_dirty_all(), >> memory_region_section_get_iotlb() and >> memory_region_is_unassigned(). >> >> Remove unused cpu_tlb_update_dirty(). >> >> Fix coding style in areas to be moved. >> >> Signed-off-by: Blue Swirl > > This patch is line-wrapped. It's even worse on Patchwork. I fixed the > mail up and applied it manually with fuzz (the exec.c 46xx hunk). > > Compiles fine on openSUSE 12.1 x86_64, and so do mingw32 and mingw64 > cross builds. > > Tested-by: Andreas Färber Thanks for testing, though I could not insert the tag anymore, sorry. I applied the series. > > 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] vga: Don't switch to 1 x 1 character text screen
Thanks, applied. On Sat, Apr 28, 2012 at 19:16, Stefan Weil wrote: > Initially, vga_get_text_resolution returns a text resolution of 1 x 1 > (vga register values are 0). > > This is visible during MIPS Malta boot with SDL. It also occurs with the > i386 or x86_64 system emulation when it runs in single step mode: > > QEMU changes the size of the SDL window to the smallest possible value > which is supported by the window manager. As this is not the calculated > size, QEMU switches to scaled mode. When the BIOS or the VGA driver sets > the normal text resolution, the window stays small and displays > microscopic characters. > > Ignoring text resolutions of 1 x 1 or less avoids these problems. > A similar workaround already exists for too large resolutions. > > Signed-off-by: Stefan Weil > --- > hw/vga.c | 4 > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/vga.c b/hw/vga.c > index f80860c..5824f85 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -1327,6 +1327,10 @@ static void vga_draw_text(VGACommonState *s, int > full_update) > line_offset = s->line_offset; > > vga_get_text_resolution(s, &width, &height, &cw, &cheight); > + if ((height * width) <= 1) { > + /* better than nothing: exit if transient size is too small */ > + return; > + } > if ((height * width) > CH_ATTR_SIZE) { > /* better than nothing: exit if transient size is too big */ > return; > -- > 1.7.9 > >
Re: [Qemu-devel] [PATCH] main-loop: Calculate poll timeout using timeout argument
Thanks, applied. On Sun, Apr 29, 2012 at 17:15, Stefan Weil wrote: > The timeout argument was unused up to now, > but it can be used to reduce the poll_timeout when it is infinite > (negative value) or larger than timeout. > > Signed-off-by: Stefan Weil > --- > main-loop.c | 6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/main-loop.c b/main-loop.c > index 24cf540..eb3b6e6 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -425,7 +425,7 @@ static int os_host_main_loop_wait(uint32_t timeout) > if (nfds >= 0) { > ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0); > if (ret != 0) { > - /* TODO. */ > + timeout = 0; > } > } > > @@ -439,6 +439,10 @@ static int os_host_main_loop_wait(uint32_t timeout) > poll_fds[n_poll_fds + i].events = G_IO_IN; > } > > + if (poll_timeout < 0 || timeout < poll_timeout) { > + poll_timeout = timeout; > + } > + > qemu_mutex_unlock_iothread(); > ret = g_poll(poll_fds, n_poll_fds + w->num, poll_timeout); > qemu_mutex_lock_iothread(); > -- > 1.7.9 > >
Re: [Qemu-devel] [PATCH for-1.1 0/2] qemu-ga: Build fixes for Solaris/illumos
Thanks, applied all. On Mon, Apr 30, 2012 at 16:00, Andreas Färber wrote: > Hello, > > Here's a mini-series fixing the build on illumos like I had requested Lee to > do. > > Patch 1 introduces a Solaris-only workaround for O_ASYNC that we could > generalize for 1.2 (ifdef O_ASYNC seemed a bit risky to me on Hard Freeze eve > in case some platform uses an enum rather than preprocessor define, given that > ioctl() is consider "obsolescent" and I_SETSIG optional in POSIX). > > Patch 2 adds libraries needed to link qemu-ga on Solaris, reusing the existing > set of backwards-compatible network libraries rather than the new libxnet. > > Regards, > Andreas > > Cc: Michael Roth > Cc: Lee Essen > Cc: Stefan Hajnoczi > Cc: Blue Swirl > > Andreas Färber (2): > qemu-ga: Implement alternative to O_ASYNC > configure: Add libraries for qemu-ga on Solaris > > configure | 4 +++- > qga/channel-posix.c | 18 +- > 2 files changed, 20 insertions(+), 2 deletions(-) > > -- > 1.7.7 >
Re: [Qemu-devel] [PATCH 1/4] exec: prepare for splitting
Am 01.05.2012 13:47, schrieb Blue Swirl: > On Tue, May 1, 2012 at 11:02, Andreas Färber wrote: >> Am 22.04.2012 17:35, schrieb Blue Swirl: >>> Make s_cputlb_empty_entry 'const'. >>> >>> Rename tlb_flush_jmp_cache() to tb_flush_jmp_cache(). >>> >>> Refactor code to add cpu_tlb_reset_dirty_all(), >>> memory_region_section_get_iotlb() and >>> memory_region_is_unassigned(). >>> >>> Remove unused cpu_tlb_update_dirty(). >>> >>> Fix coding style in areas to be moved. >>> >>> Signed-off-by: Blue Swirl >> >> This patch is line-wrapped. It's even worse on Patchwork. I fixed the >> mail up and applied it manually with fuzz (the exec.c 46xx hunk). >> >> Compiles fine on openSUSE 12.1 x86_64, and so do mingw32 and mingw64 >> cross builds. >> >> Tested-by: Andreas Färber > > Thanks for testing, though I could not insert the tag anymore, sorry. > I applied the series. Then I'll stop building the second one. ;) /-F -- 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] qemu: fix cpuid eax for kvm cpu
On 04/30/2012 05:42 PM, Michael S. Tsirkin wrote: > cpuid eax should return the max leaf so that > guests can find out the valid range. > This matches Xen et al. > > Tested using -cpu kvm64. > > Signed-off-by: Michael S. Tsirkin > --- > target-i386/kvm.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index e74a9e4..c097248 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -379,7 +379,7 @@ int kvm_arch_init_vcpu(CPUX86State *env) > c->function = KVM_CPUID_SIGNATURE; > if (!hyperv_enabled()) { > memcpy(signature, "KVMKVMKVM\0\0\0", 12); > -c->eax = 0; > +c->eax = KVM_CPUID_FEATURES; > } else { > memcpy(signature, "Microsoft Hv", 12); > c->eax = HYPERV_CPUID_MIN; Should only change for -M 1.1+? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 04/30/2012 04:40 PM, Peter Maydell wrote: > On 30 April 2012 14:36, Avi Kivity wrote: > > On 04/30/2012 04:27 PM, Peter Maydell wrote: > >> On 30 April 2012 14:23, Avi Kivity wrote: > >> > IMO the best fix is to unsysbus the device and qomify it instead. This > >> > way we're 100% flexible in how we can attach it. > >> > >> You don't need to wait for QOM to grow enough features to > >> replace sysbus. If you don't like what sysbus_mmio_map() does, you > >> can always use sysbus_mmio_get_region() to get the MemoryRegion* and > >> then deal with it however you need to. This is the standard way > >> to deal with "I have a sysbus device which I want to map into my > >> custom container object". > > > > I believe that API voids you warrantee. > > I wrote it for essentially the purpose described above :-) > If you're the owner of the sysbus device in question then it's > entirely fine as you are the one deciding whether to use the > traditional map function or not. > > It's as good as we're going to get until QOM actually lets > you export memory regions and pins, at which point we can just > convert all the sysbus devices. Sure. But expect breakage if sysbus changes, for example dropping use of get_system_memory(). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 13:39, Avi Kivity wrote: > On 04/30/2012 04:40 PM, Peter Maydell wrote: >> I wrote it for essentially the purpose described above :-) >> If you're the owner of the sysbus device in question then it's >> entirely fine as you are the one deciding whether to use the >> traditional map function or not. >> >> It's as good as we're going to get until QOM actually lets >> you export memory regions and pins, at which point we can just >> convert all the sysbus devices. > > Sure. But expect breakage if sysbus changes, for example dropping use > of get_system_memory(). That's OK, I'm happy to nak sysbus patches which break this use case :-) -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 04/30/2012 04:43 PM, Anthony Liguori wrote: > On 04/30/2012 08:36 AM, Avi Kivity wrote: >> On 04/30/2012 04:27 PM, Peter Maydell wrote: >>> On 30 April 2012 14:23, Avi Kivity wrote: IMO the best fix is to unsysbus the device and qomify it instead. This way we're 100% flexible in how we can attach it. >>> >>> You don't need to wait for QOM to grow enough features to >>> replace sysbus. If you don't like what sysbus_mmio_map() does, you >>> can always use sysbus_mmio_get_region() to get the MemoryRegion* and >>> then deal with it however you need to. This is the standard way >>> to deal with "I have a sysbus device which I want to map into my >>> custom container object". >> >> I believe that API voids you warrantee. > > All that a "QOM" conversion would do is eliminate the use of sysbus > and derive the object directly from DeviceState. Then, you would map > the MemoryRegion exported by the device directly. > > So sysbus_mmio_get_region() seems like the right API to use. > I think you're right. The real difference is that there is no longer an implied container region (which is just get_system_memory() in current sysbus). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 03:41 PM, Peter Maydell wrote: > On 1 May 2012 13:39, Avi Kivity wrote: > > On 04/30/2012 04:40 PM, Peter Maydell wrote: > >> I wrote it for essentially the purpose described above :-) > >> If you're the owner of the sysbus device in question then it's > >> entirely fine as you are the one deciding whether to use the > >> traditional map function or not. > >> > >> It's as good as we're going to get until QOM actually lets > >> you export memory regions and pins, at which point we can just > >> convert all the sysbus devices. > > > > Sure. But expect breakage if sysbus changes, for example dropping use > > of get_system_memory(). > > That's OK, I'm happy to nak sysbus patches which break this > use case :-) sysbus should just die. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 13:42, Avi Kivity wrote: > sysbus should just die. Totally agreed. It's not going to go quietly though... -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 04/30/2012 04:40 PM, Mark Cave-Ayland wrote: > On 30/04/12 14:23, Avi Kivity wrote: > > Hi Avi, > >>> My understanding based upon this is that it would be impossible to >>> register a different parent MemoryRegion without duplicating the init >>> function for all shared devices which seems undesirable :( >> >> What are the requirements? You need a different catch-all mmio handler >> for each slot? Or would one catch-all mmio handler for all sysbus >> devices suffice? > > A single catch-all for all sysbus devices would suffice, however I'm > thinking it makes sense to have one MemoryRegion per slot so that the > devices can register onto the bus using their "slot relative" address > to allow for potentially moving hardware between slots. Ok, so you have a hierarchy: bus -> slot -> devices in slot? That hierarchy is present in the hardware too, or that's how I interpret "slot relative addresses". > >>> The only solution I can think of is to make sysbus_mmio_map() more >>> intelligent so that instead of blindly adding the device to the "root" >>> MemoryRegion, it traverses down the MemoryRegion hierarchy starting >>> from the root to the furtherest leaf node it can find based upon the >>> specified address and then adds the new subregion there. Hence if I >>> add my SBus memory regions first before call the various peripheral >>> init functions, everything should end up in the correct part of the >>> memory tree. >>> >> >> This solution attempts to reconstruct the memory hierarchy from the >> address, instead of maintaining it in the device tree. > > So I guess that is bad... Well, it's a lot of work -> bad. > >>> I believe this should preserve compatibility for existing sysbus API >>> users with just a single "root" MemoryRegion, however due to lack of >>> any documentation related to sysbus I'm not sure if this would break >>> other platforms or maybe even violate one of its key design features? >> >> IMO the best fix is to unsysbus the device and qomify it instead. This >> way we're 100% flexible in how we can attach it. > > That's interesting - I didn't realise that sysbus is a legacy > interface with respect to QOM. Maybe it's just wishful thinking on my part. But I always saw sysbus as a wrong interface - it corresponds to no real bus and doesn't allow hierarchy, unlike our pci or even isa implementations. > Is there any documentation related to this? Then again, converting all > of the devices over to QOM and testing that it doesn't break all > platforms/busses suddenly becomes a huge job... > You can just follow Peter's suggestion, although qomification would be preferable IMO. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 03:43 PM, Peter Maydell wrote: > On 1 May 2012 13:42, Avi Kivity wrote: > > sysbus should just die. > > Totally agreed. It's not going to go quietly though... Not if you keep suggesting workarounds when I tell unsuspecting developers to qomify their devices. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] qemu: fix cpuid eax for kvm cpu
On Tue, May 01, 2012 at 03:38:43PM +0300, Avi Kivity wrote: > On 04/30/2012 05:42 PM, Michael S. Tsirkin wrote: > > cpuid eax should return the max leaf so that > > guests can find out the valid range. > > This matches Xen et al. > > > > Tested using -cpu kvm64. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > target-i386/kvm.c |2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > > index e74a9e4..c097248 100644 > > --- a/target-i386/kvm.c > > +++ b/target-i386/kvm.c > > @@ -379,7 +379,7 @@ int kvm_arch_init_vcpu(CPUX86State *env) > > c->function = KVM_CPUID_SIGNATURE; > > if (!hyperv_enabled()) { > > memcpy(signature, "KVMKVMKVM\0\0\0", 12); > > -c->eax = 0; > > +c->eax = KVM_CPUID_FEATURES; > > } else { > > memcpy(signature, "Microsoft Hv", 12); > > c->eax = HYPERV_CPUID_MIN; > > Should only change for -M 1.1+? I don't think we should: it's a bug fix and we don't try to be bug for bug compatible unless fixing the bug actually affects guest behaviour. > -- > error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 13:48, Avi Kivity wrote: > On 05/01/2012 03:43 PM, Peter Maydell wrote: >> On 1 May 2012 13:42, Avi Kivity wrote: >> > sysbus should just die. >> >> Totally agreed. It's not going to go quietly though... > > Not if you keep suggesting workarounds when I tell unsuspecting > developers to qomify their devices. When QOM supports (1) exporting gpio signals and (2) exporting memory regions, then it will be providing the main things that sysbus provides. (Sysbus today is essentially "I need a device that exports some memory regions and some I/O pins".) At that point it will be sensible to say "convert your sysbus devices to QOM". Until QOM is actually able to provide the functionality, it's not a workable replacement. -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 03:49 PM, Peter Maydell wrote: > On 1 May 2012 13:48, Avi Kivity wrote: > > On 05/01/2012 03:43 PM, Peter Maydell wrote: > >> On 1 May 2012 13:42, Avi Kivity wrote: > >> > sysbus should just die. > >> > >> Totally agreed. It's not going to go quietly though... > > > > Not if you keep suggesting workarounds when I tell unsuspecting > > developers to qomify their devices. > > When QOM supports (1) exporting gpio signals and (2) > exporting memory regions, then it will be providing the > main things that sysbus provides. (Sysbus today is essentially > "I need a device that exports some memory regions and some > I/O pins".) At that point it will be sensible to say "convert > your sysbus devices to QOM". Until QOM is actually able to > provide the functionality, it's not a workable replacement. Doesn't property (or however it's phrased) provide the functionality for memory? But I agree. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] qemu: fix cpuid eax for kvm cpu
On 05/01/2012 03:49 PM, Michael S. Tsirkin wrote: > On Tue, May 01, 2012 at 03:38:43PM +0300, Avi Kivity wrote: > > On 04/30/2012 05:42 PM, Michael S. Tsirkin wrote: > > > cpuid eax should return the max leaf so that > > > guests can find out the valid range. > > > This matches Xen et al. > > > > > > Tested using -cpu kvm64. > > > > > > Signed-off-by: Michael S. Tsirkin > > > --- > > > target-i386/kvm.c |2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > > > index e74a9e4..c097248 100644 > > > --- a/target-i386/kvm.c > > > +++ b/target-i386/kvm.c > > > @@ -379,7 +379,7 @@ int kvm_arch_init_vcpu(CPUX86State *env) > > > c->function = KVM_CPUID_SIGNATURE; > > > if (!hyperv_enabled()) { > > > memcpy(signature, "KVMKVMKVM\0\0\0", 12); > > > -c->eax = 0; > > > +c->eax = KVM_CPUID_FEATURES; > > > } else { > > > memcpy(signature, "Microsoft Hv", 12); > > > c->eax = HYPERV_CPUID_MIN; > > > > Should only change for -M 1.1+? > > I don't think we should: it's a bug fix and we don't try to be bug for > bug compatible unless fixing the bug actually affects guest behaviour. > I agree. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] KVM call agenda for May, Thrusday 1st
Peter Portante wrote: > On Mon, Apr 30, 2012 at 7:58 AM, Anthony Liguori > wrote: > > On 04/30/2012 04:17 AM, Juan Quintela wrote: > > > > Hi > > Please send in any agenda items you are interested in > covering. > > > > FYI, I won't be able to attend as I'll be at the LF End User > Summit. > > Regards, > > Anthony Liguori > > > Thanks, Juan. > > > > > > Just for clarity, is this for Tuesday, May 1st, or Thursday, May 3rd? Tuesday (got cancelled). How I typed Thrusday is still a mistery. Later, Juan.
Re: [Qemu-devel] [PATCH v2 4/5] configure: check for supported Python 2.x versions
Stefan Hajnoczi writes: > The tracetool code requires Python 2.4, which was released in 2004. > Check for a supported Python version so we can give a clear error > message. > Signed-off-by: Stefan Hajnoczi > --- > configure |7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > diff --git a/configure b/configure > index 25697bb..0e6fbbc 100755 > --- a/configure > +++ b/configure > @@ -1247,9 +1247,10 @@ fi > # Note that if the Python conditional here evaluates True we will exit > # with status 1 which is a shell 'false' value. > -if ! "$python" -c 'import sys; sys.exit(sys.version_info[0] >= 3)'; then > - echo "Python 2 required but '$python' is version 3 or better." > - echo "Use --python=/path/to/python to specify a Python 2." > +if ! "$python" -c 'import sys; sys.version_info < (2,4) or sys.version_info > >= (3,)'; then > + echo "Cannot use '$python', Python 2.4 or later is required." > + echo "Note that Python 3 or later is not yet supported." > + echo "Use --python=/path/to/python to specify a supported Python." >exit 1 > fi Shouldn't this be using "sys.exit(...version checks...)" ? Lluis -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [PATCH v2 5/5] tracetool: avoid pkgutil.iter_modules() Python 2.7 function
Stefan Hajnoczi writes: > The pkgutil.iter_modules() function provides a way to enumerate child > modules. Unfortunately it's missing in Python <2.7 so we must implement > similar behavior ourselves. > Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova > --- > scripts/tracetool/backend/__init__.py |8 ++-- > scripts/tracetool/format/__init__.py |8 ++-- > 2 files changed, 12 insertions(+), 4 deletions(-) > diff --git a/scripts/tracetool/backend/__init__.py > b/scripts/tracetool/backend/__init__.py > index 34b7ed8..be43472 100644 > --- a/scripts/tracetool/backend/__init__.py > +++ b/scripts/tracetool/backend/__init__.py > @@ -37,7 +37,7 @@ __maintainer__ = "Stefan Hajnoczi" > __email__ = "stefa...@linux.vnet.ibm.com" > -import pkgutil > +import os > import tracetool > @@ -45,7 +45,11 @@ import tracetool > def get_list(): > """Get a list of (name, description) pairs.""" > res = [("nop", "Tracing disabled.")] > -for _, modname, _ in pkgutil.iter_modules(tracetool.backend.__path__): > +modnames = [] > +for filename in os.listdir(tracetool.backend.__path__[0]): > +if filename.endswith('.py') and filename != '__init__.py': > +modnames.append(filename.rsplit('.', 1)[0]) > +for modname in modnames: > module = tracetool.try_import("tracetool.backend." + modname) > # just in case; should never fail unless non-module files are put > there > diff --git a/scripts/tracetool/format/__init__.py > b/scripts/tracetool/format/__init__.py > index 0e4baf0..3c2a0d8 100644 > --- a/scripts/tracetool/format/__init__.py > +++ b/scripts/tracetool/format/__init__.py > @@ -41,7 +41,7 @@ __maintainer__ = "Stefan Hajnoczi" > __email__ = "stefa...@linux.vnet.ibm.com" > -import pkgutil > +import os > import tracetool > @@ -49,7 +49,11 @@ import tracetool > def get_list(): > """Get a list of (name, description) pairs.""" > res = [] > -for _, modname, _ in pkgutil.iter_modules(tracetool.format.__path__): > +modnames = [] > +for filename in os.listdir(tracetool.format.__path__[0]): > +if filename.endswith('.py') and filename != '__init__.py': > +modnames.append(filename.rsplit('.', 1)[0]) > +for modname in modnames: > module = tracetool.try_import("tracetool.format." + modname) > # just in case; should never fail unless non-module files are put > there > -- > 1.7.10 -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 11:19 AM, Blue Swirl wrote: > On Mon, Apr 30, 2012 at 16:39, Artyom Tarasenko wrote: >> Tried to boot QEMU Niagara machine with the firmware from the >> OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) >> , and it dies very early. >> The reason: in translate.c >> >> #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) >> #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) >> >> and the dc->mem_idx is initialized like this: >> >> if (env1->tl > 0) { >> return MMU_NUCLEUS_IDX; >> } else if (cpu_hypervisor_mode(env1)) { >> return MMU_HYPV_IDX; >> } else if (cpu_supervisor_mode(env1)) { >> return MMU_KERNEL_IDX; >> } else { >> return MMU_USER_IDX; >> } >> >> Which seems to be conceptually incorrect. After reset tl == MAXTL, but >> still super- and hyper-visor bits are set, so both supervisor(dc) and >> hypervisor(dc) must return 1 which is impossible in the current >> implementation. > > I don't think this is needed. The MMU index tells which TLB is used > for guest virtual to host address translations, during tl == MAXTL we > want to use hypervisor mode translations. Is just about the address translations? I thought it affects TB cache as well, which has to be invalidated between nucleus, super- and hypervisor switches? >> What would be the proper way to fix it? Make mem_idx bitmap, add two >> more variables to DisasContext, or ...? >>
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 08:01 AM, Avi Kivity wrote: On 05/01/2012 03:49 PM, Peter Maydell wrote: On 1 May 2012 13:48, Avi Kivity wrote: On 05/01/2012 03:43 PM, Peter Maydell wrote: On 1 May 2012 13:42, Avi Kivity wrote: sysbus should just die. Totally agreed. It's not going to go quietly though... Not if you keep suggesting workarounds when I tell unsuspecting developers to qomify their devices. When QOM supports (1) exporting gpio signals and This is trivial. It'll come in as soon as 1.2 opens up. If folks want to start working on a branch with it: https://github.com/aliguori/qemu/tree/qom-pin.4 (2) exporting memory regions, then it will be providing the main things that sysbus provides. This is a little tricky. Here's the problems I've encountered so far: a) A lot of devices need the equivalent of it_shift. it_shift affects how addresses are decoded and the size of the memory region. it_shift usually needs to be a device property. Since we need to know the size of the memory region to initialize it, we need to know the value of it_shift before we can initialize it which means we have to delay the initialization of the mmemory region until realize. I think a nice fix would be to make it_shift as memory region mutator and allow it to be set after initialization. b) There's some duplication in MemoryRegions with respect to QOM. Memory regions want to have a name but with QOM they'll be addressable via a path. I go back and forth about how aggressively we want to refactor MemoryRegions. If someone wants to spend some time converting MemoryRegion to QOM, that would certainly be helpful. So far, I've been avoiding it but we'll eventually need to do it. Regards, Anthony Liguori
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 01:57 AM, Blue Swirl wrote: On Mon, Apr 30, 2012 at 13:52, Mark Cave-Ayland wrote: On 30/04/12 14:27, Peter Maydell wrote: Hi Peter, IMO the best fix is to unsysbus the device and qomify it instead. This way we're 100% flexible in how we can attach it. You don't need to wait for QOM to grow enough features to replace sysbus. If you don't like what sysbus_mmio_map() does, you Oh - so does this mean that QOM is not feature-complete? At least 'Pin' class to replace GPIOs and IRQs is not in HEAD (but Anthony already has a tree somewhere) and I can't remember what was the plan for buses. I'm planning on pushing bus support in before I freeze 1.1. Regards, Anthony Liguori
Re: [Qemu-devel] KVM call agenda for May, Thrusday 1st
Juan Quintela wrote: > Hi > > Please send in any agenda items you are interested in covering. Iht was Tuesday 1st. Anthony said that he can't attend, there are no topics yet, and half of Europe (at least) is on holiday. So, call got cancelled. Later, Juan. /me learning to type Tuesday instead of Thrusday.
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 13:33, Artyom Tarasenko wrote: > On Tue, May 1, 2012 at 11:19 AM, Blue Swirl wrote: >> On Mon, Apr 30, 2012 at 16:39, Artyom Tarasenko wrote: >>> Tried to boot QEMU Niagara machine with the firmware from the >>> OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) >>> , and it dies very early. >>> The reason: in translate.c >>> >>> #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) >>> #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) >>> >>> and the dc->mem_idx is initialized like this: >>> >>> if (env1->tl > 0) { >>> return MMU_NUCLEUS_IDX; >>> } else if (cpu_hypervisor_mode(env1)) { >>> return MMU_HYPV_IDX; >>> } else if (cpu_supervisor_mode(env1)) { >>> return MMU_KERNEL_IDX; >>> } else { >>> return MMU_USER_IDX; >>> } >>> >>> Which seems to be conceptually incorrect. After reset tl == MAXTL, but >>> still super- and hyper-visor bits are set, so both supervisor(dc) and >>> hypervisor(dc) must return 1 which is impossible in the current >>> implementation. >> >> I don't think this is needed. The MMU index tells which TLB is used >> for guest virtual to host address translations, during tl == MAXTL we >> want to use hypervisor mode translations. > > Is just about the address translations? I thought it affects TB cache > as well, which has to be invalidated between nucleus, super- and > hypervisor switches? This is (should be) handled by cpu_get_tb_cpu_state(). But it looks like only supervisor mode (PS_PRIV) is taken into account, not others. Also when D/I MMU is enabled or disabled, TLB is flushed, so there's no need to store these bits. I'm not sure TL is interesting either. >>> What would be the proper way to fix it? Make mem_idx bitmap, add two >>> more variables to DisasContext, or ...? >>>
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 11:25 AM, Blue Swirl wrote: > On Mon, Apr 30, 2012 at 17:38, Artyom Tarasenko wrote: >> On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber wrote: >>> Am 30.04.2012 18:39, schrieb Artyom Tarasenko: Tried to boot QEMU Niagara machine with the firmware from the OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) , and it dies very early. The reason: in translate.c #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) and the dc->mem_idx is initialized like this: if (env1->tl > 0) { return MMU_NUCLEUS_IDX; } else if (cpu_hypervisor_mode(env1)) { return MMU_HYPV_IDX; } else if (cpu_supervisor_mode(env1)) { return MMU_KERNEL_IDX; } else { return MMU_USER_IDX; } Which seems to be conceptually incorrect. After reset tl == MAXTL, but still super- and hyper-visor bits are set, so both supervisor(dc) and hypervisor(dc) must return 1 which is impossible in the current implementation. What would be the proper way to fix it? Make mem_idx bitmap, add two more variables to DisasContext, or ...? Some other findings/questions: /* Sun4v generic Niagara machine */ { .default_cpu_model = "Sun UltraSparc T1", .console_serial_base = 0xfff0c2c000ULL, Where is this address coming from? The OpenSPARC Niagara machine has a "dumb serial" at 0x1f1000ULL. And the biggest issue: UA2005 (as well as UA2007) describe a totally different format for a MMU TTE entry than the one sun4u CPU are using. I think the best way to handle it would be splitting off Niagara machine, and #defining MMU bits differently for sun4u and sun4v machines. Do we the cases in qemu where more than two (qemu-system-xxx and qemu-system-xxx64) binaries are produced? Would the name qemu-system-sun4v fit the naming convention? >>> >>> We have such a case for ppc (ppcemb) and it is kind of a maintenance >>> nightmare - I'm working towards getting rid of it with my QOM CPU work. >>> Better avoid it for sparc in the first place. >>> >>> Instead, you should add a callback function pointer to SPARCCPUClass >>> that you initialize based on CPU model so that is behaves differently at >>> runtime rather than at compile time. >>> Or if it's just about the class_init then after the Hard Freeze I can >>> start polishing my subclasses for sparc so that you can add a special >>> class_init for Niagara. >> >> But this would mean that the defines from >> #define TTE_NFO_BIT (1ULL << 60) >> to >> #define TTE_PGSIZE(tte) (((tte) >> 61) & 3ULL) >> >> inclusive would need to be replaced with functions and variables? >> Sounds like a further performance regression for sun4u? > > There could be parallel definitions for sun4u (actually UltraSparc-III > onwards the MMU is again different) and sun4v. > > At tlb_fill(), different implementations can be selected based on MMU > model. For ASI accesses, we can add conditional code but for higher > performance, some checks can be moved to translation time. Can be done, but what is the gain of having it runtime configurable? >> And would it be possible to have a different register set for an >> inherited SPARCCPUClass ? >> Also trap handling and related cpu registers behavior has to be quite >> different. > > Not really, we already handle some (but not all cases). You mean by not handling AG? The UA2005 doesn't have IG and MG either, but that's the easy part. Instead it has a few register banks switchable with the GL register pointer. cpu_change_pstate should probably have another parameter (new_GL) which is only valid for sun4v. And, depending on a trap type, env->htba has to be taken instead of env->tbr. To me it looks like at the end do_interrupt will have less common parts between sun4u and sun4v than specific ones. >>> Helpers such as cpu_hypervisor_mode() will need to be changed to take a >>> SPARCCPU *cpu rather than CPUSPARCState *env argument; as an interim >>> solution sparc_env_get_cpu() can be used. (examples on the list for sh4) >>> >>> Andreas >>> >>> -- >>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >> -- Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 14:50, Anthony Liguori wrote: >> On 05/01/2012 03:49 PM, Peter Maydell wrote: >>> When QOM supports (1) exporting gpio signals and > > This is trivial. It'll come in as soon as 1.2 opens up. You need to get it through code review first... (just as a random example, this commit: https://github.com/aliguori/qemu/commit/46483049813fca2c23fb7cd04eeab89905500c4c is going down the wrong path because it's incorrectly introducing knowledge of device internals to the utility i8259_init() function.) -- PMM
Re: [Qemu-devel] [PATCH] sdl: Avoid unnecessary resizing of the display surface
Il 01/05/2012 07:53, Stefan Weil ha scritto: > If neither width nor height changes, nothing has to be done. > > Cc: Anthony Liguori > Signed-off-by: Stefan Weil > --- > > This patch improves SDL for any host (for example with remote X displays), > but the main reason why I wrote it was another problem: > > On w32 / w64 hosts, qemu-system-arm has a deadlock when it calls > sdl_resize_displaysurface during Linux boot. One thread waits for > a critical region, another thread waits inside SDL_SetVideoMode. > > The patch avoids this problem. Paolo, maybe you have an idea what > could cause the deadlock. No idea---honestly, my w32 interest is limited to replacing hacks with proper solutions. :) It turns out that in many cases you can fix bugs along the way and that helps having people look at your patches... Paolo
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 10:35 AM, Andreas Färber wrote: > Am 30.04.2012 19:38, schrieb Artyom Tarasenko: >> On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber wrote: >>> Am 30.04.2012 18:39, schrieb Artyom Tarasenko: Tried to boot QEMU Niagara machine with the firmware from the OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) , and it dies very early. The reason: in translate.c #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) and the dc->mem_idx is initialized like this: if (env1->tl > 0) { return MMU_NUCLEUS_IDX; } else if (cpu_hypervisor_mode(env1)) { return MMU_HYPV_IDX; } else if (cpu_supervisor_mode(env1)) { return MMU_KERNEL_IDX; } else { return MMU_USER_IDX; } Which seems to be conceptually incorrect. After reset tl == MAXTL, but still super- and hyper-visor bits are set, so both supervisor(dc) and hypervisor(dc) must return 1 which is impossible in the current implementation. What would be the proper way to fix it? Make mem_idx bitmap, add two more variables to DisasContext, or ...? Some other findings/questions: /* Sun4v generic Niagara machine */ { .default_cpu_model = "Sun UltraSparc T1", .console_serial_base = 0xfff0c2c000ULL, Where is this address coming from? The OpenSPARC Niagara machine has a "dumb serial" at 0x1f1000ULL. And the biggest issue: UA2005 (as well as UA2007) describe a totally different format for a MMU TTE entry than the one sun4u CPU are using. I think the best way to handle it would be splitting off Niagara machine, and #defining MMU bits differently for sun4u and sun4v machines. Do we the cases in qemu where more than two (qemu-system-xxx and qemu-system-xxx64) binaries are produced? Would the name qemu-system-sun4v fit the naming convention? >>> >>> We have such a case for ppc (ppcemb) and it is kind of a maintenance >>> nightmare - I'm working towards getting rid of it with my QOM CPU work. >>> Better avoid it for sparc in the first place. >>> >>> Instead, you should add a callback function pointer to SPARCCPUClass >>> that you initialize based on CPU model so that is behaves differently at >>> runtime rather than at compile time. >>> Or if it's just about the class_init then after the Hard Freeze I can >>> start polishing my subclasses for sparc so that you can add a special >>> class_init for Niagara. >> >> But this would mean that the defines from >> #define TTE_NFO_BIT (1ULL << 60) >> to >> #define TTE_PGSIZE(tte) (((tte) >> 61) & 3ULL) >> >> inclusive would need to be replaced with functions and variables? >> Sounds like a further performance regression for sun4u? > > First of all, I wouldn't use the term performance regression for a > target such as sparc64 that has never really worked before. ;-) Well, it works good enough for me. ;-) Also until the recent PCI changes it was able to boot HelenOS out of the box. > I don't know the details of the SPARC implementation or architecture. > What I am telling you is that there are ways to implement sun4u and > sun4v inside one executable; whether you use a new function or extend a > macro with an additional parameter is up to you and Blue. My guess is > that unlike ARM you won't need to mix different cores at runtime for the > time being. > > In ARM we have a macro IS_M() that distinguishes between the Cortex-M > and Cortex-A families rather than moving Cortex-M to its own executable. > The Freescale Vybrid MCUs are going to combine the two in one SoC, i.e. > in one qemu-system-arm executable simultaneously. > >> And would it be possible to have a different register set for an >> inherited SPARCCPUClass ? > > You can add new state fields to an inherited SPARCCPU. Can the inheritance hierarchy have any depth? Then we could think of having SPARC->(SPARC32,SPARC64) and SPARC64 -> (sun4u, sun4v, Fujitsu). But I still don't see the advantages of having it configurable at runtime. > If you need them in TCG as offsets from env, then CPUSPARCState might be > an easier place to add them but there you cannot distinguish between the > models. > > To an inherited SPARCCPUClass you can add fields for saving constant, > e.g., reset values for registers of the model. > > The unanswered question is whether SPARC should follow the x86 model > with declarative (and user-specified) definitions (in that case > extending SPARCCPUClass) or the ARM model with imperative per-model init > functions (in that case probably not extending SPARCCPU at all). > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- Regards, Artyom Tarasenko solaris/sp
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 13:54, Artyom Tarasenko wrote: > On Tue, May 1, 2012 at 11:25 AM, Blue Swirl wrote: >> On Mon, Apr 30, 2012 at 17:38, Artyom Tarasenko wrote: >>> On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber wrote: Am 30.04.2012 18:39, schrieb Artyom Tarasenko: > Tried to boot QEMU Niagara machine with the firmware from the > OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html ) > , and it dies very early. > The reason: in translate.c > > #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX) > #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX) > > and the dc->mem_idx is initialized like this: > > if (env1->tl > 0) { > return MMU_NUCLEUS_IDX; > } else if (cpu_hypervisor_mode(env1)) { > return MMU_HYPV_IDX; > } else if (cpu_supervisor_mode(env1)) { > return MMU_KERNEL_IDX; > } else { > return MMU_USER_IDX; > } > > Which seems to be conceptually incorrect. After reset tl == MAXTL, but > still super- and hyper-visor bits are set, so both supervisor(dc) and > hypervisor(dc) must return 1 which is impossible in the current > implementation. > > What would be the proper way to fix it? Make mem_idx bitmap, add two > more variables to DisasContext, or ...? > > Some other findings/questions: > > /* Sun4v generic Niagara machine */ > { > .default_cpu_model = "Sun UltraSparc T1", > .console_serial_base = 0xfff0c2c000ULL, > > Where is this address coming from? The OpenSPARC Niagara machine has a > "dumb serial" at 0x1f1000ULL. > > And the biggest issue: UA2005 (as well as UA2007) describe a totally > different format for a MMU TTE entry than the one sun4u CPU are using. > I think the best way to handle it would be splitting off Niagara > machine, and #defining MMU bits differently for sun4u and sun4v > machines. > > Do we the cases in qemu where more than two (qemu-system-xxx and > qemu-system-xxx64) binaries are produced? > Would the name qemu-system-sun4v fit the naming convention? We have such a case for ppc (ppcemb) and it is kind of a maintenance nightmare - I'm working towards getting rid of it with my QOM CPU work. Better avoid it for sparc in the first place. Instead, you should add a callback function pointer to SPARCCPUClass that you initialize based on CPU model so that is behaves differently at runtime rather than at compile time. Or if it's just about the class_init then after the Hard Freeze I can start polishing my subclasses for sparc so that you can add a special class_init for Niagara. >>> >>> But this would mean that the defines from >>> #define TTE_NFO_BIT (1ULL << 60) >>> to >>> #define TTE_PGSIZE(tte) (((tte) >> 61) & 3ULL) >>> >>> inclusive would need to be replaced with functions and variables? >>> Sounds like a further performance regression for sun4u? >> >> There could be parallel definitions for sun4u (actually UltraSparc-III >> onwards the MMU is again different) and sun4v. >> >> At tlb_fill(), different implementations can be selected based on MMU >> model. For ASI accesses, we can add conditional code but for higher >> performance, some checks can be moved to translation time. > > Can be done, but what is the gain of having it runtime configurable? I was thinking of code like this in: switch (env->mmu_model) { case MMU_US2: return tlb_fill_us2(..); case MMU_US3: return tlb_fill_us3(..); case MMU_US4: return tlb_fill_us4(..); case MMU_T1: return tlb_fill_t1(..); case MMU_T2: return tlb_fill_t2(..); } The perfomance cost shouldn't be too high. Alternatively a function pointer could be set up. > >>> And would it be possible to have a different register set for an >>> inherited SPARCCPUClass ? >>> Also trap handling and related cpu registers behavior has to be quite >>> different. >> >> Not really, we already handle some (but not all cases). > > You mean by not handling AG? The UA2005 doesn't have IG and MG either, > but that's the easy part. Instead it has a few register banks > switchable with the GL register pointer. Yes, we can always provide the register bank, older models just access some of those. > cpu_change_pstate should probably have another parameter (new_GL) > which is only valid for sun4v. > And, depending on a trap type, env->htba has to be taken instead of > env->tbr. To me it looks like at the end do_interrupt will have less > common parts between sun4u and sun4v than specific ones. Same as tlb_fill(), switch() or function pointer. The functions are different. This is unavoidable (unless maybe in the future the TLB handling can be pushed partially higher so mmu_idx parameters can be eliminated) and the performance cost is not great. > Helpers such as cpu_hypervisor_mode() will need to be c
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 09:00 AM, Peter Maydell wrote: On 1 May 2012 14:50, Anthony Liguori wrote: On 05/01/2012 03:49 PM, Peter Maydell wrote: When QOM supports (1) exporting gpio signals and This is trivial. It'll come in as soon as 1.2 opens up. You need to get it through code review first... Obviously. (just as a random example, this commit: https://github.com/aliguori/qemu/commit/46483049813fca2c23fb7cd04eeab89905500c4c is going down the wrong path because it's incorrectly introducing knowledge of device internals to the utility i8259_init() function.) Eh? Do you mean: -qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]); +pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); There are three ways to do this: 1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]); 2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); 3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]); Having converted a lot of devices by hand, I prefer (2). I'd like to move to a model of: typedef struct PICDevice { DeviceState parent; Pin int_out[0]; MemoryRegion io; /*< private >*/ PICState *priv; } PICDevice; This let's us use (2), while still embedding devices and keeping the gory state details private to the implementation. Regards, Anthony Liguori -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 04:50 PM, Anthony Liguori wrote: > On 05/01/2012 08:01 AM, Avi Kivity wrote: >> On 05/01/2012 03:49 PM, Peter Maydell wrote: >>> On 1 May 2012 13:48, Avi Kivity wrote: On 05/01/2012 03:43 PM, Peter Maydell wrote: > On 1 May 2012 13:42, Avi Kivity wrote: >> sysbus should just die. > > Totally agreed. It's not going to go quietly though... Not if you keep suggesting workarounds when I tell unsuspecting developers to qomify their devices. >>> >>> When QOM supports (1) exporting gpio signals and > > This is trivial. It'll come in as soon as 1.2 opens up. If folks > want to start working on a branch with it: > > https://github.com/aliguori/qemu/tree/qom-pin.4 > >>> (2) exporting memory regions, then it will be providing the >>> main things that sysbus provides. > > This is a little tricky. Here's the problems I've encountered so far: > > a) A lot of devices need the equivalent of it_shift. it_shift affects > how addresses are decoded and the size of the memory region. it_shift > usually needs to be a device property. > > Since we need to know the size of the memory region to initialize it, > we need to know the value of it_shift before we can initialize it > which means we have to delay the initialization of the mmemory region > until realize. > > I think a nice fix would be to make it_shift as memory region mutator > and allow it to be set after initialization. Indeed I wanted to make it_shift as part of the memory core. But a mutator? It doesn't change in real hardware, so this looks artificial. So far all mutators really change at runtime. What is the problem with delaying region initialization until realize? > > b) There's some duplication in MemoryRegions with respect to QOM. > Memory regions want to have a name but with QOM they'll be addressable > via a path. I go back and forth about how aggressively we want to > refactor MemoryRegions. These days region names are purely for debugging. The ABI bit was moved to a separate function. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 09:09 AM, Avi Kivity wrote: On 05/01/2012 04:50 PM, Anthony Liguori wrote: On 05/01/2012 08:01 AM, Avi Kivity wrote: On 05/01/2012 03:49 PM, Peter Maydell wrote: On 1 May 2012 13:48, Avi Kivity wrote: On 05/01/2012 03:43 PM, Peter Maydell wrote: On 1 May 2012 13:42, Avi Kivity wrote: sysbus should just die. Totally agreed. It's not going to go quietly though... Not if you keep suggesting workarounds when I tell unsuspecting developers to qomify their devices. When QOM supports (1) exporting gpio signals and This is trivial. It'll come in as soon as 1.2 opens up. If folks want to start working on a branch with it: https://github.com/aliguori/qemu/tree/qom-pin.4 (2) exporting memory regions, then it will be providing the main things that sysbus provides. This is a little tricky. Here's the problems I've encountered so far: a) A lot of devices need the equivalent of it_shift. it_shift affects how addresses are decoded and the size of the memory region. it_shift usually needs to be a device property. Since we need to know the size of the memory region to initialize it, we need to know the value of it_shift before we can initialize it which means we have to delay the initialization of the mmemory region until realize. I think a nice fix would be to make it_shift as memory region mutator and allow it to be set after initialization. Indeed I wanted to make it_shift as part of the memory core. But a mutator? It doesn't change in real hardware, so this looks artificial. So far all mutators really change at runtime. QOM has a concept of initialization and realize. You can change properties after initialization but not before realize. So as long as it_shift can be set after initialization but before realize (which I think is roughly memory_region_add_subregion) it doesn't need to be a mutator. What is the problem with delaying region initialization until realize? We need to initialize the MemoryRegion in order to expose it as a property. We want to do that during initialize. Here's an example: qom-create isa-i8259 foo qom-set /peripheral/foo/io it_shift 1 qom-set /peripheral/foo/realize true For this to work, it_shift needs to be a QOM property of the "io" MemoryRegion. The MemoryRegion needs to be created in instance_init. b) There's some duplication in MemoryRegions with respect to QOM. Memory regions want to have a name but with QOM they'll be addressable via a path. I go back and forth about how aggressively we want to refactor MemoryRegions. These days region names are purely for debugging. The ABI bit was moved to a separate function. Fair enough. BTW, in the branch I've posted, I've got a number of memory API conversions or removal of legacy interfaces. Regards, Anthony Liguori
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 15:06, Anthony Liguori wrote: > Do you mean: > > - qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]); > + pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); > > There are three ways to do this: > > 1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]); No good unless you're autogenerating all those accessor functions. > 2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); Exposes the whole of the struct (including all the private memmbers) to anything that wants to use it. As you say later on in the email, this requires splitting everything up into two structs, one for "publicly accessible" and one for "private implementation"...which your series doesn't seem to do at the moment. > 3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]); This is a bit verbose. Something more like pin_connect(s, "int_out[0]", self, "int_set[2]"); would be a bit less longwinded. Or even connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]"); (No, this doesn't do compile time type checking. I don't think that's a problem particularly, or at least not enough of one to justify not doing it this way. The object model we have is dynamic and does things at runtime...) -- PMM
Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode
On 01.05.2012, at 11:15, Blue Swirl wrote: > On Mon, Apr 30, 2012 at 11:51, Alexander Graf wrote: >> >> On 30.04.2012, at 12:45, Alexander Graf wrote: >> >>> >>> On 22.04.2012, at 15:26, Blue Swirl wrote: >>> Add an explicit CPUPPCState parameter instead of relying on AREG0 and rename op_helper.c (which only contains load and store helpers) to mem_helper.c. Remove AREG0 swapping in tlb_fill(). Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores. >>> >>> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm >>> trying to debug it down, but worst case I'll omit this patch set for 1.1. >> >> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. >> It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path >> is missing completely. >> >> This actually makes me less confident that this is a change we want for 1.1. >> I'll remove the patches from the queue. > > Meh. It should be perfectly OK to apply all patches except the last > one which enables the AREG0 free mode. Yeah, that's what I did at first, but that still didn't get me into usable user space inside a ppc64 guest. Right now I don't have the time to track down if it's due to your patches and if so, why. > Also the problem with last > patch is not in the patch itself but PPC TCG host support, which by > the way is probably also broken for AREG0 free Sparc and Alpha, so I'd > really like to see them in 1.1. I do agree on the first part. We need to make sure to test sparc and alpha targets on unusual host platforms and fix them there. But why do we need to also break ppc along the way? > There should be plenty of time to fix > bugs in PPC TCG support during the freeze. Since this is a non user visible feature (in fact, looking at the emitted asm code it'll be more of a slowdown than anything), I'd rather like to keep 1.1 stable and get this into git right after the release split. It's really not going against your patches. In fact, I really do like them - especially the cleanups. But this feature is pretty invasive and at least I do run ppc-on-ppc tcg, so we should be able to hammer out all bugs until the next release :). The whole AREG0 thing could also use some optimization love... Alex
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 05:15 PM, Anthony Liguori wrote: >>> I think a nice fix would be to make it_shift as memory region mutator >>> and allow it to be set after initialization. >> >> Indeed I wanted to make it_shift as part of the memory core. But a >> mutator? It doesn't change in real hardware, so this looks artificial. >> So far all mutators really change at runtime. > > > QOM has a concept of initialization and realize. You can change > properties after initialization but not before realize. > > So as long as it_shift can be set after initialization but before > realize (which I think is roughly memory_region_add_subregion) it > doesn't need to be a mutator. Ok, good. > >> What is the problem with delaying region initialization until realize? > > We need to initialize the MemoryRegion in order to expose it as a > property. We want to do that during initialize. Here's an example: > > qom-create isa-i8259 foo > qom-set /peripheral/foo/io it_shift 1 > qom-set /peripheral/foo/realize true > > For this to work, it_shift needs to be a QOM property of the "io" > MemoryRegion. The MemoryRegion needs to be created in instance_init. So it looks like we need two phase initialization for memory regions as well? Not so pretty. > >>> b) There's some duplication in MemoryRegions with respect to QOM. >>> Memory regions want to have a name but with QOM they'll be addressable >>> via a path. I go back and forth about how aggressively we want to >>> refactor MemoryRegions. >> >> These days region names are purely for debugging. The ABI bit was moved >> to a separate function. > > Fair enough. > > BTW, in the branch I've posted, I've got a number of memory API > conversions or removal of legacy interfaces. Nice. But you use get_system_io(), which is bad. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 09:20 AM, Peter Maydell wrote: On 1 May 2012 15:06, Anthony Liguori wrote: Do you mean: -qdev_connect_gpio_out(&dev->qdev, 0, irq_set[2]); +pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); There are three ways to do this: 1) pin_connect_qemu_irq(i8259_get_int_out(s), irq_set[2]); No good unless you're autogenerating all those accessor functions. 2) pin_connect_qemu_irq(&s->int_out[0], irq_set[2]); Exposes the whole of the struct (including all the private memmbers) to anything that wants to use it. As you say later on in the email, this requires splitting everything up into two structs, one for "publicly accessible" and one for "private implementation"...which your series doesn't seem to do at the moment. 3) pin_connect_qemu_irq(PIN(object_get_child(s, "int_out[0]")), irq_set[2]); This is a bit verbose. Something more like pin_connect(s, "int_out[0]", self, "int_set[2]"); This is incorrect, it would have to be: Error *err = NULL; if (pin_connect(s, "in_out[0]", self, "int_set[2]", &err)) { error_propagate(errp, err); return; } would be a bit less longwinded. Or even connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]"); Not checking for failure is not an option. (No, this doesn't do compile time type checking. I don't think that's a problem particularly, or at least not enough of one to justify not doing it this way. The object model we have is dynamic and does things at runtime...) Correctness is more important to me than brevity. And really, we should focus on killing things like i8259_init(). These types of functions should go away in favor of proper modeling of SoCs/SuperIO chips. Regards, Anthony Liguori -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 09:26 AM, Avi Kivity wrote: On 05/01/2012 05:15 PM, Anthony Liguori wrote: I think a nice fix would be to make it_shift as memory region mutator and allow it to be set after initialization. Indeed I wanted to make it_shift as part of the memory core. But a mutator? It doesn't change in real hardware, so this looks artificial. So far all mutators really change at runtime. QOM has a concept of initialization and realize. You can change properties after initialization but not before realize. So as long as it_shift can be set after initialization but before realize (which I think is roughly memory_region_add_subregion) it doesn't need to be a mutator. Ok, good. What is the problem with delaying region initialization until realize? We need to initialize the MemoryRegion in order to expose it as a property. We want to do that during initialize. Here's an example: qom-create isa-i8259 foo qom-set /peripheral/foo/io it_shift 1 qom-set /peripheral/foo/realize true For this to work, it_shift needs to be a QOM property of the "io" MemoryRegion. The MemoryRegion needs to be created in instance_init. So it looks like we need two phase initialization for memory regions as well? Yes, exactly. Converting MemoryRegion to QOM will give it a two phase initialization FWIW. Not so pretty. It's unavoidable because we're dealing with a graph. The connections between objects may form loops which effectively means that we have dependency loops. That means we need to be able to create all objects first and then establish the links between them. This is why we need two stage initialization. b) There's some duplication in MemoryRegions with respect to QOM. Memory regions want to have a name but with QOM they'll be addressable via a path. I go back and forth about how aggressively we want to refactor MemoryRegions. These days region names are purely for debugging. The ABI bit was moved to a separate function. Fair enough. BTW, in the branch I've posted, I've got a number of memory API conversions or removal of legacy interfaces. Nice. But you use get_system_io(), which is bad. Only in the device setup code. That will all eventually get moved into a SuperIO chip. The PC code needs massive amounts of refactoring. Regards, Anthony Liguori
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 16:09, Anthony Liguori wrote: > On 05/01/2012 09:20 AM, Peter Maydell wrote: >> This is a bit verbose. Something more like >> pin_connect(s, "int_out[0]", self, "int_set[2]"); > > This is incorrect, it would have to be: > > Error *err = NULL; > > if (pin_connect(s, "in_out[0]", self, "int_set[2]", &err)) { > error_propagate(errp, err); > return; > > } > >> would be a bit less longwinded. Or even >> connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]"); > > > Not checking for failure is not an option. The assumption is that failure to connect is a fatal error, and we can happily just assert()/hw_error()/etc. (Obviously we would provide APIs for the occasional use case that really did want to do "connect if possible and do something else in case of failure", but that's definitely not the common case.) >> (No, this doesn't do compile time type checking. I don't >> think that's a problem particularly, or at least not enough >> of one to justify not doing it this way. The object model we >> have is dynamic and does things at runtime...) > > Correctness is more important to me than brevity. > > And really, we should focus on killing things like i8259_init(). Functions like i8259_init() exist precisely because QOM/qdev don't provide brevity and people trying to use these devices do in fact value brevity. That's why I want the standard native "connect this thing to this other thing" function to be short and simple. -- PMM
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 05/01/2012 10:20 AM, Peter Maydell wrote: On 1 May 2012 16:09, Anthony Liguori wrote: On 05/01/2012 09:20 AM, Peter Maydell wrote: This is a bit verbose. Something more like pin_connect(s, "int_out[0]", self, "int_set[2]"); This is incorrect, it would have to be: Error *err = NULL; if (pin_connect(s, "in_out[0]", self, "int_set[2]",&err)) { error_propagate(errp, err); return; } would be a bit less longwinded. Or even connect(TYPE_PIN, s, "int_out[0]", self, "int_set[2]"); Not checking for failure is not an option. The assumption is that failure to connect is a fatal error, and we can happily just assert()/hw_error()/etc. So that means that we have a bug from someone miss-typing, instead of your hotplug attempt failing with an error, your entire guest is destroyed. That doesn't sound very nice to me. Device initialization should never exit() (unless you really hit a fatal error like OOM). (No, this doesn't do compile time type checking. I don't think that's a problem particularly, or at least not enough of one to justify not doing it this way. The object model we have is dynamic and does things at runtime...) Correctness is more important to me than brevity. And really, we should focus on killing things like i8259_init(). Functions like i8259_init() exist precisely because QOM/qdev don't provide brevity and people trying to use these devices do in fact value brevity. No, they exist because we aren't modeling correctly. i8259_init() is doing a few different things at once. Once you split things between init and realize, you no longer have long chunks of redundant code. That's why I want the standard native "connect this thing to this other thing" function to be short and simple. With my previous proposal, it's just: s->irq_in = &b->int_out[0]; This is why I like exposing public members in structures. It's brief and safe. Regards, Anthony Liguori -- PMM
[Qemu-devel] [PATCH for-1.1] linux-user: fix emulation of /proc/self/maps
From: Alexander Graf Improve the emulation of /proc/self/maps by reading the underlying host maps file and passing lines through with addresses adjusted to be guest addresses. This is necessary to avoid false triggers of the glibc check that a format string containing '%n' is not in writable memory. (For an example see the bug reported in https://bugs.launchpad.net/qemu-linaro/+bug/947888 where gpg aborts.) Signed-off-by: Alexander Graf Signed-off-by: Peter Maydell --- I've been running this patch in qemu-linaro for a bit and it's also in Alex's SuSE QEMU 1.0 tree, but I hadn't realised until now that it hadn't made it into master. This should go into 1.1 because otherwise we'll regress compared to 1.0, because glibc can cope with "/proc/self/maps doesn't exist" but not with "exists but has almost no content", which is what the current master gives you. linux-user/syscall.c | 42 +- 1 files changed, 41 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 7128618..9a86e00 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4866,13 +4866,53 @@ int get_osversion(void) static int open_self_maps(void *cpu_env, int fd) { +#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) TaskState *ts = ((CPUArchState *)cpu_env)->opaque; +#endif +FILE *fp; +char *line = NULL; +size_t len = 0; +ssize_t read; + +fp = fopen("/proc/self/maps", "r"); +if (fp == NULL) { +return -EACCES; +} +while ((read = getline(&line, &len, fp)) != -1) { +int fields, dev_maj, dev_min, inode; +uint64_t min, max, offset; +char flag_r, flag_w, flag_x, flag_p; +char path[512] = ""; +fields = sscanf(line, "%"PRIx64"-%"PRIx64" %c%c%c%c %"PRIx64" %x:%x %d" +" %512s", &min, &max, &flag_r, &flag_w, &flag_x, +&flag_p, &offset, &dev_maj, &dev_min, &inode, path); + +if ((fields < 10) || (fields > 11)) { +continue; +} +if (!strncmp(path, "[stack]", 7)) { +continue; +} +if (h2g_valid(min) && h2g_valid(max)) { +dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx +" %c%c%c%c %08" PRIx64 " %02x:%02x %d%s%s\n", +h2g(min), h2g(max), flag_r, flag_w, +flag_x, flag_p, offset, dev_maj, dev_min, inode, +path[0] ? " " : "", path); +} +} + +free(line); +fclose(fp); + +#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0 [stack]\n", (unsigned long long)ts->info->stack_limit, (unsigned long long)(ts->stack_base + (TARGET_PAGE_SIZE - 1)) & TARGET_PAGE_MASK, -(unsigned long long)ts->stack_base); +(unsigned long long)0); +#endif return 0; } -- 1.7.1
Re: [Qemu-devel] [PATCH v2 4/5] configure: check for supported Python 2.x versions
On Tue, May 1, 2012 at 2:20 PM, Lluís Vilanova wrote: > Stefan Hajnoczi writes: > >> The tracetool code requires Python 2.4, which was released in 2004. >> Check for a supported Python version so we can give a clear error >> message. > >> Signed-off-by: Stefan Hajnoczi >> --- >> configure | 7 --- >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> diff --git a/configure b/configure >> index 25697bb..0e6fbbc 100755 >> --- a/configure >> +++ b/configure >> @@ -1247,9 +1247,10 @@ fi > >> # Note that if the Python conditional here evaluates True we will exit >> # with status 1 which is a shell 'false' value. >> -if ! "$python" -c 'import sys; sys.exit(sys.version_info[0] >= 3)'; then >> - echo "Python 2 required but '$python' is version 3 or better." >> - echo "Use --python=/path/to/python to specify a Python 2." >> +if ! "$python" -c 'import sys; sys.version_info < (2,4) or sys.version_info >> >= (3,)'; then >> + echo "Cannot use '$python', Python 2.4 or later is required." >> + echo "Note that Python 3 or later is not yet supported." >> + echo "Use --python=/path/to/python to specify a supported Python." >> exit 1 >> fi > > Shouldn't this be using "sys.exit(...version checks...)" ? Yes, thanks for pointing it out. Stefan
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 1 May 2012 16:26, Anthony Liguori wrote: > On 05/01/2012 10:20 AM, Peter Maydell wrote: >> The assumption is that failure to connect is a fatal error, >> and we can happily just assert()/hw_error()/etc. > > So that means that we have a bug from someone miss-typing, instead of your > hotplug attempt failing with an error, your entire guest is destroyed. That > doesn't sound very nice to me. If you're trying to hotplug a buggily implemented device then all bets are off, really. > Device initialization should never exit() (unless you really hit a fatal > error like OOM). > > (No, this doesn't do compile time type checking. I don't think that's a problem particularly, or at least not enough of one to justify not doing it this way. The object model we have is dynamic and does things at runtime...) >>> >>> >>> Correctness is more important to me than brevity. >>> >>> And really, we should focus on killing things like i8259_init(). >> >> >> Functions like i8259_init() exist precisely because >> QOM/qdev don't provide brevity and people trying to >> use these devices do in fact value brevity. > > > No, they exist because we aren't modeling correctly. Most of them are simply convenience functions that just do "create, configure, wire up, init". (The ones that do more than that need fixing anyway.) > i8259_init() is doing a few different things at once. > Once you split things between init and realize, you no longer > have long chunks of redundant code. ...you have redundant code scattered in multiple places instead? >> That's why >> I want the standard native "connect this thing to this >> other thing" function to be short and simple. > > > With my previous proposal, it's just: > > s->irq_in = &b->int_out[0]; > > This is why I like exposing public members in structures. It's brief and > safe. Obvious question: why isn't property setting done this way, then? Surely it's equally brief and safe to say cpu->level = def_level; rather than object_property_set_int(OBJECT(cpu), def->level, "level", &error); I don't particularly object to this sort of "public struct vs private struct" object model, it's just not what you've actually implemented. -- PMM
[Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd
Libvirt can take advantage of SELinux to restrict the QEMU process and prevent it from opening files that it should not have access to. This improves security because it prevents the attacker from escaping the QEMU process if they manage to gain control. NFS has been a pain point for SELinux because it does not support labels (which I believe are stored in extended attributes). In other words, it's not possible to use SELinux goodness on QEMU when image files are located on NFS. Today we have to allow QEMU access to any file on the NFS export rather than restricting specifically to the image files that the guest requires. File descriptor passing is a solution to this problem and might also come in handy elsewhere. Libvirt or another external process chooses files which QEMU is allowed to access and provides just those file descriptors - QEMU cannot open the files itself. This series adds the -open-hook-fd command-line option. Whenever QEMU needs to open an image file it sends a request over the given UNIX domain socket. The response includes the file descriptor or an errno on failure. Please see the patches for details on the protocol. The -open-hook-fd approach allows QEMU to support file descriptor passing without changing -drive. It also supports snapshot_blkdev and other commands that re-open image files. Anthony Liguori wrote most of these patches. I added a demo -open-hook-fd server and added some small fixes. Since Anthony is traveling right now I'm sending the RFC for discussion. Anthony Liguori (3): block: add open() wrapper that can be hooked by libvirt block: add new command line parameter that and protocol description block: plumb up open-hook-fd option Stefan Hajnoczi (2): osdep: add qemu_recvmsg() wrapper Example -open-hook-fd server block.c | 107 ++ block.h |2 + block/raw-posix.c | 18 +++ block/raw-win32.c |2 +- block/vdi.c |2 +- block/vmdk.c |6 +-- block/vpc.c |2 +- block/vvfat.c |4 +- block_int.h | 12 + osdep.c | 46 + qemu-common.h |2 + qemu-options.hx | 42 +++ test-fd-passing.c | 147 + vl.c |3 ++ 14 files changed, 378 insertions(+), 17 deletions(-) create mode 100644 test-fd-passing.c -- 1.7.10
[Qemu-devel] [RFC 2/5] block: add new command line parameter that and protocol description
From: Anthony Liguori Signed-off-by: Anthony Liguori Signed-off-by: Stefan Hajnoczi --- qemu-options.hx | 42 ++ 1 file changed, 42 insertions(+) diff --git a/qemu-options.hx b/qemu-options.hx index a169792..ccf4d1d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2724,6 +2724,48 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "-qtest-log LOG specify tracing options\n", QEMU_ARCH_ALL) +DEF("open-hook-fd", HAS_ARG, QEMU_OPTION_open_hook_fd, +"-open-hook-fd \n" +"delegate opens to external process using \n", QEMU_ARCH_ALL) +STEXI +@item -open-hook-fd @var{fd} +@findex -open-hook-fd +Delegates open()s to an external process using @var to communicate commands. +@var should be an open Unix Domain socket pipe that file descriptors can be +received from. The protocol the socket uses is a simple request/response initiated +by the client. All integers are in host byte order. It is assumed that this protocol +is only ever used on the same physical machine. It is currently defined as: + +u32 message_size +u32 command +u8 payload[message_size - 8] + +The contents of payload depend on command. Currently the following commands are +defined: + +1. QEMU_OPEN (1) + +The full message will be: + +u32 message_size +u32 command = 1 +u32 flags (O_ flags defined by libc) +u32 mode (mode_t flags as defined by libc) +u16 filename_len; +u8 filename[filename_len] + +The server will then respond with: + +u32 message_size +u32 command = 1 +s32 result + +If result is < 0, the value will be negated errno value as defined in errno.h. If +result is equal to 0, then there will also be a file descriptor available via SCM_RIGHTS +in the extended sendmsg data. + +ETEXI + HXCOMM This is the last statement. Insert new options before this line! STEXI @end table -- 1.7.10
[Qemu-devel] [RFC 3/5] block: plumb up open-hook-fd option
From: Anthony Liguori Implement the open hook UNIX domain socket protocol and accept passed file descriptors. Signed-off-by: Anthony Liguori Signed-off-by: Stefan Hajnoczi --- block.c | 107 - block.h |2 + block/raw-posix.c |2 +- qemu-options.hx |4 +- vl.c |3 ++ 5 files changed, 114 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index f9c4633..d3bf443 100644 --- a/block.c +++ b/block.c @@ -31,6 +31,7 @@ #include "qemu-coroutine.h" #include "qmp-commands.h" #include "qemu-timer.h" +#include "qemu_socket.h" #ifdef CONFIG_BSD #include @@ -102,9 +103,113 @@ static BlockDriverState *bs_snapshots; /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; +static int remote_file_fd = -1; + +void remote_file_open_init(int fd) +{ +remote_file_fd = fd; +} + +typedef struct OpenRequest +{ +uint32_t message_len; +uint32_t type; +uint32_t flags; +uint32_t mode; +uint32_t filename_len; +uint8_t filename[0]; +} OpenRequest; + +typedef struct OpenResponse +{ +uint32_t message_len; +uint32_t type; +int32_t result; +} OpenResponse; + int file_open(const char *filename, int flags, mode_t mode) { -return open(filename, flags, mode); +#ifdef _WIN32 +return qemu_open(filename, flags, mode); +#else +struct msghdr msg = { NULL, }; +struct cmsghdr *cmsg; +struct iovec iov[1]; +union { +struct cmsghdr cmsg; +char control[CMSG_SPACE(sizeof(int))]; +} msg_control; +ssize_t ret; +uint8_t buffer[1024]; +OpenRequest *req = (void *)buffer; +OpenResponse *rsp = (void *)buffer; + +if (remote_file_fd == -1) { +return qemu_open(filename, flags, mode); +} + +req->filename_len = strlen(filename); +req->message_len = sizeof(OpenRequest) + req->filename_len; +req->type = 1; /* OpenRequest */ +req->flags = flags; +req->mode = mode; + +if (req->message_len > sizeof(buffer)) { +errno = EFAULT; +return -1; +} +memcpy(req->filename, filename, req->filename_len); + +do { +ret = send(remote_file_fd, req, req->message_len, 0); +} while (ret == -1 && errno == EINTR); +if (ret != req->message_len) { +errno = EPIPE; +return -1; +} + +iov[0].iov_base = buffer; +iov[0].iov_len = sizeof(buffer); + +msg.msg_iov = iov; +msg.msg_iovlen = 1; +msg.msg_control = &msg_control; +msg.msg_controllen = sizeof(msg_control); + +do { +ret = recvmsg(remote_file_fd, &msg, 0); +} while (ret == -1 && errno == EINTR); +if (ret != sizeof(OpenResponse)) { +errno = EPIPE; +return -1; +} + +rsp = (void *)buffer; +if (rsp->result < 0) { +errno = -rsp->result; +return -1; +} + +for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { +int fd; + +if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) || +cmsg->cmsg_level != SOL_SOCKET || +cmsg->cmsg_type != SCM_RIGHTS) { +continue; +} + +fd = *((int *)CMSG_DATA(cmsg)); +if (fd < 0) { +continue; +} + +return fd; +} + +errno = ENOENT; +return -1; +#endif } #ifdef _WIN32 diff --git a/block.h b/block.h index f163e54..b8b78c7 100644 --- a/block.h +++ b/block.h @@ -336,6 +336,8 @@ int bdrv_img_create(const char *filename, const char *fmt, void bdrv_set_buffer_alignment(BlockDriverState *bs, int align); void *qemu_blockalign(BlockDriverState *bs, size_t size); +void remote_file_open_init(int fd); + #define BDRV_SECTORS_PER_DIRTY_CHUNK 2048 void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable); diff --git a/block/raw-posix.c b/block/raw-posix.c index b6bc6bc..9946e5f 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -208,7 +208,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, s->open_flags |= O_DSYNC; s->fd = -1; -fd = qemu_open(filename, s->open_flags, 0644); +fd = file_open(filename, s->open_flags, 0644); if (fd < 0) { ret = -errno; if (ret == -EROFS) diff --git a/qemu-options.hx b/qemu-options.hx index ccf4d1d..0c54cd5 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2730,8 +2730,8 @@ DEF("open-hook-fd", HAS_ARG, QEMU_OPTION_open_hook_fd, STEXI @item -open-hook-fd @var{fd} @findex -open-hook-fd -Delegates open()s to an external process using @var to communicate commands. -@var should be an open Unix Domain socket pipe that file descriptors can be +Delegates open()s to an external process using @var{fd} to communicate commands. +@var{fd} should be an open Unix Domain socket pipe that file descriptors can be received from. The protocol the socket uses is a simple request/response initiated by the client. All integers are in host byte order. It
[Qemu-devel] [RFC 4/5] osdep: add qemu_recvmsg() wrapper
Usually we need to set O_CLOEXEC, which is platform-specific. Add a wrapper like qemu_open() but for qemu_recvmsg(). Signed-off-by: Stefan Hajnoczi --- block.c |5 + osdep.c | 46 ++ qemu-common.h |2 ++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index d3bf443..e66d64f 100644 --- a/block.c +++ b/block.c @@ -176,10 +176,7 @@ int file_open(const char *filename, int flags, mode_t mode) msg.msg_control = &msg_control; msg.msg_controllen = sizeof(msg_control); -do { -ret = recvmsg(remote_file_fd, &msg, 0); -} while (ret == -1 && errno == EINTR); -if (ret != sizeof(OpenResponse)) { +if (qemu_recvmsg(remote_file_fd, &msg, 0) != sizeof(OpenResponse)) { errno = EPIPE; return -1; } diff --git a/osdep.c b/osdep.c index 3e6bada..834e78f 100644 --- a/osdep.c +++ b/osdep.c @@ -103,6 +103,52 @@ int qemu_open(const char *name, int flags, ...) } /* + * Receive a message from a socket + * + * This behaves like recvmsg(2) except that EINTR is handled internally and + * never returned. Passed file descriptors will have O_CLOEXEC set. + */ +ssize_t qemu_recvmsg(int fd, struct msghdr *msg, int flags) +{ +ssize_t ret; + +#ifdef MSG_CMSG_CLOEXEC +/* Receive file descriptors with O_CLOEXEC */ +flags |= MSG_CMSG_CLOEXEC; +#endif + +do { +ret = recvmsg(fd, msg, flags); +} while (ret == -1 && errno == EINTR); +if (ret < 0) { +return ret; +} + +#ifndef MSG_CMSG_CLOEXEC +/* As a fallback, set O_CLOEXEC in a way that is not thread-safe */ +{ +struct cmsghdr *cmsg; +for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) { +int new_fd; + +if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) || +cmsg->cmsg_level != SOL_SOCKET || +cmsg->cmsg_type != SCM_RIGHTS) { +continue; +} + +new_fd = *((int *)CMSG_DATA(cmsg)); +if (new_fd >= 0) { +qemu_set_cloexec(new_fd); +} +} +} +#endif + +return ret; +} + +/* * A variant of write(2) which handles partial write. * * Return the number of bytes transferred. diff --git a/qemu-common.h b/qemu-common.h index 50f659a..e3a6c4d 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -184,6 +184,8 @@ const char *path(const char *pathname); void *qemu_oom_check(void *ptr); int qemu_open(const char *name, int flags, ...); +struct msghdr; +ssize_t qemu_recvmsg(int fd, struct msghdr *msg, int flags); ssize_t qemu_write_full(int fd, const void *buf, size_t count) QEMU_WARN_UNUSED_RESULT; ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags) -- 1.7.10
[Qemu-devel] [RFC 1/5] block: add open() wrapper that can be hooked by libvirt
From: Anthony Liguori Signed-off-by: Anthony Liguori Signed-off-by: Stefan Hajnoczi --- block.c |5 + block/raw-posix.c | 16 block/raw-win32.c |2 +- block/vdi.c |2 +- block/vmdk.c |6 +++--- block/vpc.c |2 +- block/vvfat.c |4 ++-- block_int.h | 12 8 files changed, 33 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index 43c794c..f9c4633 100644 --- a/block.c +++ b/block.c @@ -102,6 +102,11 @@ static BlockDriverState *bs_snapshots; /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; +int file_open(const char *filename, int flags, mode_t mode) +{ +return open(filename, flags, mode); +} + #ifdef _WIN32 static int is_windows_drive_prefix(const char *filename) { diff --git a/block/raw-posix.c b/block/raw-posix.c index 2d1bc13..b6bc6bc 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -568,7 +568,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, +fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) { result = -errno; @@ -741,7 +741,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if ( bsdPath[ 0 ] != '\0' ) { strcat(bsdPath,"s0"); /* some CDs don't have a partition 0 */ -fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); +fd = file_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE, 0); if (fd < 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { @@ -798,7 +798,7 @@ static int fd_open(BlockDriverState *bs) #endif return -EIO; } -s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK); +s->fd = file_open(bs->filename, s->open_flags & ~O_NONBLOCK, 0); if (s->fd < 0) { s->fd_error_time = get_clock(); s->fd_got_error = 1; @@ -872,7 +872,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_BINARY); +fd = file_open(filename, O_WRONLY | O_BINARY, 0); if (fd < 0) return -errno; @@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename) if (strstart(filename, "/dev/fd", NULL)) prio = 50; -fd = open(filename, O_RDONLY | O_NONBLOCK); +fd = file_open(filename, O_RDONLY | O_NONBLOCK, 0); if (fd < 0) { goto out; } @@ -1003,7 +1003,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) close(s->fd); s->fd = -1; } -fd = open(bs->filename, s->open_flags | O_NONBLOCK); +fd = file_open(bs->filename, s->open_flags | O_NONBLOCK, 0); if (fd >= 0) { if (ioctl(fd, FDEJECT, 0) < 0) perror("FDEJECT"); @@ -1053,7 +1053,7 @@ static int cdrom_probe_device(const char *filename) int prio = 0; struct stat st; -fd = open(filename, O_RDONLY | O_NONBLOCK); +fd = file_open(filename, O_RDONLY | O_NONBLOCK, 0); if (fd < 0) { goto out; } @@ -1177,7 +1177,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s->fd >= 0) close(s->fd); -fd = open(bs->filename, s->open_flags, 0644); +fd = file_open(bs->filename, s->open_flags, 0644); if (fd < 0) { s->fd = -1; return -EIO; diff --git a/block/raw-win32.c b/block/raw-win32.c index e4b0b75..a64651b 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -255,7 +255,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, +fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) return -EIO; diff --git a/block/vdi.c b/block/vdi.c index 119d3c7..c8be34b 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -648,7 +648,7 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, +fd = file_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644); if (fd < 0) { return -errno; diff --git a/block/vmdk.c b/block/vmdk.c index 18e9b4c..1cf86f1 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1161,7 +1161,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, VMDK4Header header; uint32_t tmp, magic, grains, gd_size, gt_size, gt_count; -fd = open( +fd = file_open( filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644); @@ -1484,12 +1484,12 @@ static int vmdk_create(const char *filename, QEMUOpti
[Qemu-devel] [RFC 5/5] Example -open-hook-fd server
This patch implements a demo server for the new -open-hook-fd feature. It opens any filename given to it by QEMU and therefore adds no true security. But it serves as a good debugging tool to see what requests QEMU is making. $ gcc -o test-fd-passing -Wall test-fd-passing.c $ ./test-fd-passing path/to/my/vm.img Try: (qemu) change ide1-cd0 path/to/a/cdrom.iso Signed-off-by: Stefan Hajnoczi --- test-fd-passing.c | 147 + 1 file changed, 147 insertions(+) create mode 100644 test-fd-passing.c diff --git a/test-fd-passing.c b/test-fd-passing.c new file mode 100644 index 000..43b2e86 --- /dev/null +++ b/test-fd-passing.c @@ -0,0 +1,147 @@ +/* + * QEMU -open-hook-fd test server + * + * Copyright IBM, Corp. 2012 + * + * Authors: + * Stefan Hajnoczi + * + * This work is licensed under the terms of the GNU LGPL, version 2 or later. + * See the COPYING.LIB file in the top-level directory. + * + * gcc -o test-fd-passing -Wall test-fd-passing.c + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +typedef struct { +uint32_t message_len; +uint32_t type; +uint32_t flags; +uint32_t mode; +uint32_t filename_len; +uint8_t filename[0]; +} OpenRequest; + +typedef struct { +uint32_t message_len; +uint32_t type; +int32_t result; +} OpenResponse; + +int main(int argc, char **argv) +{ +if (argc != 2) { +fprintf(stderr, "usage: %s \n", argv[0]); +return EXIT_FAILURE; +} + +int fds[2]; +if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) { +perror("socketpair"); +return EXIT_FAILURE; +} + +char *fdstr = NULL; +asprintf(&fdstr, "%d", fds[1]); + +char *drivestr = NULL; +asprintf(&drivestr, "file=%s,cache=none,if=virtio", argv[1]); + +char *child_argv[] = { +"qemu-system-x86_64", +"-enable-kvm", +"-m", "1024", +"-drive", drivestr, +"-open-hook-fd", fdstr, +NULL, +}; + +pid_t child_pid; +if (posix_spawn(&child_pid, "x86_64-softmmu/qemu-system-x86_64", +NULL, NULL, child_argv, environ) != 0) { +fprintf(stderr, "posix_spawn failed\n"); +return EXIT_FAILURE; +} +free(drivestr); +free(fdstr); +close(fds[1]); + +for (;;) { +OpenRequest req; +char filename[1024]; + +if (recv(fds[0], &req, sizeof(req), 0) != sizeof(req)) { +perror("recv"); +return EXIT_FAILURE; +} + +if (req.type != 1 /* OpenRequest */) { +fprintf(stderr, "Expected request type 1, got %u\n", req.type); +return EXIT_FAILURE; +} + +if (req.filename_len > sizeof(filename) - 1) { +fprintf(stderr, "Filename length too large (%u)\n", +req.filename_len); +return EXIT_FAILURE; +} + +if (recv(fds[0], filename, req.filename_len, 0) != req.filename_len) { +perror("recv"); +return EXIT_FAILURE; +} +filename[req.filename_len] = '\0'; + +fprintf(stderr, "open(\"%s\", %#x, %#o) = ", +filename, req.flags, req.mode); + +int fd, ret; +fd = ret = open(filename, req.flags, req.mode); + +fprintf(stderr, "%d (errno %d)\n", ret, errno); + +OpenResponse resp = { +.message_len = sizeof(resp), +.type = 1, +.result = ret < 0 ? -errno : 0, +}; +struct iovec iov = { +.iov_base = &resp, +.iov_len = sizeof(resp), +}; +char buf[CMSG_SPACE(sizeof(int))]; +struct msghdr msg = { +.msg_iov = &iov, +.msg_iovlen = 1, +}; +if (ret >= 0) { +msg.msg_control = buf; +msg.msg_controllen = sizeof(buf); + +struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg); +cmsg->cmsg_level = SOL_SOCKET; +cmsg->cmsg_type = SCM_RIGHTS; +cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + +memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); +} + +ret = sendmsg(fds[0], &msg, 0); +if (ret < 0) { +perror("sendmsg"); +return EXIT_FAILURE; +} +close(fd); +} +} -- 1.7.10
[Qemu-devel] [PATCH v3 2/5] tracetool: use Python 2.4-compatible __import__() arguments
In Python 2.5 keyword arguments were added to __import__(). Avoid using them to achieve Python 2.4 compatibility. Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova --- scripts/tracetool/__init__.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 74fe21b..49858c9 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -204,7 +204,7 @@ def try_import(mod_name, attr_name = None, attr_default = None): object or attribute value. """ try: -module = __import__(mod_name, fromlist=["__package__"]) +module = __import__(mod_name, globals(), locals(), ["__package__"]) if attr_name is None: return True, module return True, getattr(module, str(attr_name), attr_default) -- 1.7.10
Re: [Qemu-devel] [PATCH v3 5/5] configure: check for supported Python 2.x versions
Stefan Hajnoczi writes: > The tracetool code requires Python 2.4, which was released in 2004. > Check for a supported Python version so we can give a clear error > message. > Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova > --- > configure |7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > diff --git a/configure b/configure > index 25697bb..ccffd87 100755 > --- a/configure > +++ b/configure > @@ -1247,9 +1247,10 @@ fi > # Note that if the Python conditional here evaluates True we will exit > # with status 1 which is a shell 'false' value. > -if ! "$python" -c 'import sys; sys.exit(sys.version_info[0] >= 3)'; then > - echo "Python 2 required but '$python' is version 3 or better." > - echo "Use --python=/path/to/python to specify a Python 2." > +if ! "$python" -c 'import sys; sys.exit(sys.version_info < (2,4) or > sys.version_info >= (3,))'; then > + echo "Cannot use '$python', Python 2.4 or later is required." > + echo "Note that Python 3 or later is not yet supported." > + echo "Use --python=/path/to/python to specify a supported Python." >exit 1 > fi > -- > 1.7.10 -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [libvirt] [RFC 5/5] Example -open-hook-fd server
On Tue, May 1, 2012 at 4:31 PM, Stefan Hajnoczi wrote: > This patch implements a demo server for the new -open-hook-fd feature. > It opens any filename given to it by QEMU and therefore adds no true > security. But it serves as a good debugging tool to see what requests > QEMU is making. > > $ gcc -o test-fd-passing -Wall test-fd-passing.c > $ ./test-fd-passing path/to/my/vm.img > > Try: > > (qemu) change ide1-cd0 path/to/a/cdrom.iso > > Signed-off-by: Stefan Hajnoczi > --- > test-fd-passing.c | 147 > + > 1 file changed, 147 insertions(+) > create mode 100644 test-fd-passing.c Although this whole series is RFC, this particular patch is not meant for qemu.git - it's just an example of how to talk to QEMU. It's not 100% portable or QEMU coding style but please don't worry, this will not dirty our beautiful source tree :D. Stefan
[Qemu-devel] [PATCH v3 5/5] configure: check for supported Python 2.x versions
The tracetool code requires Python 2.4, which was released in 2004. Check for a supported Python version so we can give a clear error message. Signed-off-by: Stefan Hajnoczi --- configure |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 25697bb..ccffd87 100755 --- a/configure +++ b/configure @@ -1247,9 +1247,10 @@ fi # Note that if the Python conditional here evaluates True we will exit # with status 1 which is a shell 'false' value. -if ! "$python" -c 'import sys; sys.exit(sys.version_info[0] >= 3)'; then - echo "Python 2 required but '$python' is version 3 or better." - echo "Use --python=/path/to/python to specify a Python 2." +if ! "$python" -c 'import sys; sys.exit(sys.version_info < (2,4) or sys.version_info >= (3,))'; then + echo "Cannot use '$python', Python 2.4 or later is required." + echo "Note that Python 3 or later is not yet supported." + echo "Use --python=/path/to/python to specify a supported Python." exit 1 fi -- 1.7.10
Re: [Qemu-devel] [Bug 992067] Re: Windows 2008R2 very slow cold boot when >4GB memory
Hi, I was having this problem with qemu 0.15 , (win2008R2 x64 > 4GB). guest with 32gb ram take around 40min to boot. (When windows fill memory at boot, memory grow slowly) but now with last qemu kvm git (proxmox 2.0 distrib), it's working fine. (can tell for qemu 1.0) - Mail original - De: "Gleb Natapov" À: anth...@codemonkey.ws Cc: matth...@base3.com.au, qemu-devel@nongnu.org Envoyé: Mardi 1 Mai 2012 10:03:44 Objet: Re: [Qemu-devel] [Bug 992067] Re: Windows 2008R2 very slow cold boot when >4GB memory On Mon, Apr 30, 2012 at 06:07:55PM -, Anthony Liguori wrote: > This should be resolved by using Hyper-V relaxed timers which is in the > latest development version of QEMU. You would need to add -cpu > host,+hv_relaxed to the command line to verify this. > The described scenario still shouldn't happen and it doesn't for me. Is this 32 or 64 bit version of Windows. Are you running 32 bit version of the host may be (but Scientific Linux shouldn't have that)? What is your command line? > ** Changed in: qemu > Status: New => Fix Committed > > -- > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. > https://bugs.launchpad.net/bugs/992067 > > Title: > Windows 2008R2 very slow cold boot when >4GB memory > > Status in QEMU: > Fix Committed > > Bug description: > I've been having a consistent problem booting 2008R2 guests with > 4096MB of RAM or greater. On the initial boot the KVM process starts > out with a ~200MB memory allocation and will use 100% of all CPU > allocated to it. The RES memory of the KVM process slowly rises by > around 200mb every few minutes until it reaches it's memory allocation > (several hours in some cases). Whilst this is happening the guest will > usually blue screen with the message of - > > A clock interrupt was not received on a secondary processor within the > allocated time interval > > If I let the KVM process continue to run it will eventually allocate > the required memory the guest will run at full speed, usually > restarting after the blue screen and booting into startup repair. From > here you can restart it and it will boot perfectly. Once booted the > guest has no performance issues at all. > > I've tried everything I could think of. Removing PAE, playing with > huge pages, different kernels, different userspaces, different > systems, different backing file systems, different processor feature > set, with or without Virtio etc. My best theory is that the problem is > caused by Windows 2008 zeroing out all the memory on boot and > something is causing this to be held up or slowed to a crawl. The > hosts always have memory free to boot the guest and are not using swap > at all. > > Nothing so far has solved the issue. A few observations I've made about the > issue are - > Large memory 2008R2 guests seem to boot fine (or with a small delay) when > they are the first to boot on the host after a reboot > Sometimes dropping the disk cache (echo 1 > /proc/sys/vm/drop_caches) will > cause them to boot faster > > > The hosts I've tried are - > All Nehalem based (5540, 5620 and 5660) > Host ram of 48GB, 96GB and 192GB > Storage on NFS, Gluster and local (ext4, xfs and zfs) > QED, QCOW and RAW formats > Scientific Linux 6.1 with the standard kernel 2.6.32, 2.6.38 and 3.3.1 > KVM userspaces 0.12, 0.14 and (currently) 0.15.1 > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/992067/+subscriptions -- Gleb. -- -- Alexandre D erumier Ingénieur Système Fixe : 03 20 68 88 90 Fax : 03 20 68 90 81 45 Bvd du Général Leclerc 59100 Roubaix - France 12 rue Marivaux 75002 Paris - France
Re: [Qemu-devel] [PATCH v2] sdl: Add QEMU mascot icon for use with SDL
Am 01.05.2012 13:19, schrieb Andreas Färber: Am 23.04.2012 22:11, schrieb Stefan Weil: Am 13.04.2012 22:24, schrieb Stefan Weil: This is a bitmap file (32x32x4) derived from the official QEMU mascot (which was designed by Benoît Canet). I stripped the text from the SVG to get a nearly square image and converted the result to BMP without any manual optimization. The bitmap is currently used by QEMU's SDL interface and replaces the default X icon. v2: Add qemu-icon.bmp to Makefile. Signed-off-by: Stefan Weil --- Makefile | 1 + pc-bios/qemu-icon.bmp | Bin 0 -> 630 bytes 2 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 pc-bios/qemu-icon.bmp It looks kind of ugly in GNOME3, where it is scaled up to some other resolution in the top bar. IIRC Windows 7 uses 64x64 max, Mac OS X 128x128 max, GNOME 48x48(?). When we add icons for Gtk+ and Cocoa we might also want to rename the file to indicate size and/or purpose (e.g., qemu-sdl-icon.bmp / qemu-32x32-icon.bmp). For Windows, wouldn't it be a better idea to use the multi-resolution .ico format through version.rc and to avoid the generic SDL icon being used? Don't see that being set up in this patch - #ifdef it out there? Andreas Yes, there are other formats for the different hosts which look better and which are standard for the respective operating systems, but they also need special code and / or linker intructions. As far as I know, SDL supports only 32x32x4 bitmaps, so this icon is the best possible format which works on any host with SDL. I always wanted to have a QEMU specific icon for the QEMU executables (any host) and for the installer (w32/w64 only). That's why I wrote the SDL icon code and added this patch here. IMHO, improving the icon can be done when QEMU has an adequate GUI. Stefan
[Qemu-devel] [PATCH v3 4/5] tracetool: avoid pkgutil.iter_modules() Python 2.7 function
The pkgutil.iter_modules() function provides a way to enumerate child modules. Unfortunately it's missing in Python <2.7 so we must implement similar behavior ourselves. Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova --- scripts/tracetool/backend/__init__.py |8 ++-- scripts/tracetool/format/__init__.py |8 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py index 34b7ed8..be43472 100644 --- a/scripts/tracetool/backend/__init__.py +++ b/scripts/tracetool/backend/__init__.py @@ -37,7 +37,7 @@ __maintainer__ = "Stefan Hajnoczi" __email__ = "stefa...@linux.vnet.ibm.com" -import pkgutil +import os import tracetool @@ -45,7 +45,11 @@ import tracetool def get_list(): """Get a list of (name, description) pairs.""" res = [("nop", "Tracing disabled.")] -for _, modname, _ in pkgutil.iter_modules(tracetool.backend.__path__): +modnames = [] +for filename in os.listdir(tracetool.backend.__path__[0]): +if filename.endswith('.py') and filename != '__init__.py': +modnames.append(filename.rsplit('.', 1)[0]) +for modname in modnames: module = tracetool.try_import("tracetool.backend." + modname) # just in case; should never fail unless non-module files are put there diff --git a/scripts/tracetool/format/__init__.py b/scripts/tracetool/format/__init__.py index 0e4baf0..3c2a0d8 100644 --- a/scripts/tracetool/format/__init__.py +++ b/scripts/tracetool/format/__init__.py @@ -41,7 +41,7 @@ __maintainer__ = "Stefan Hajnoczi" __email__ = "stefa...@linux.vnet.ibm.com" -import pkgutil +import os import tracetool @@ -49,7 +49,11 @@ import tracetool def get_list(): """Get a list of (name, description) pairs.""" res = [] -for _, modname, _ in pkgutil.iter_modules(tracetool.format.__path__): +modnames = [] +for filename in os.listdir(tracetool.format.__path__[0]): +if filename.endswith('.py') and filename != '__init__.py': +modnames.append(filename.rsplit('.', 1)[0]) +for modname in modnames: module = tracetool.try_import("tracetool.format." + modname) # just in case; should never fail unless non-module files are put there -- 1.7.10
[Qemu-devel] [PATCH v3 3/5] tracetool: avoid str.rpartition() Python 2.5 function
The str.rpartition() function is related to str.split() and is used for splitting strings. It was introduced in Python 2.5 and therefore cannot be used in tracetool as Python 2.4 compatibility is required. Replace the code using str.rsplit(). Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova --- scripts/tracetool/__init__.py | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 49858c9..175df08 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -64,14 +64,17 @@ class Arguments: res = [] for arg in arg_str.split(","): arg = arg.strip() -parts = arg.split() -head, sep, tail = parts[-1].rpartition("*") -parts = parts[:-1] -if tail == "void": -assert len(parts) == 0 and sep == "" +if arg == 'void': continue -arg_type = " ".join(parts + [ " ".join([head, sep]).strip() ]).strip() -res.append((arg_type, tail)) + +if '*' in arg: +arg_type, identifier = arg.rsplit('*', 1) +arg_type += '*' +identifier = identifier.strip() +else: +arg_type, identifier = arg.rsplit(None, 1) + +res.append((arg_type, identifier)) return Arguments(res) def __iter__(self): -- 1.7.10
[Qemu-devel] [PATCH v3 0/5] tracetool: Python 2.4 compatibility fixes
The new Python tracetool implementation works great but does not run on older Python installations. This series takes us back to the happy days of Python 2.4, which was released in 2004. As a result tracetool should now work again on Mac OS X v10.5.8, OpenIndiana oi_151a, Solaris 10 U9, and Red Hat Enterprise Linux 5. I added a new patch which should make --list-backends work on Python <2.7 now. Thanks for everyone's help testing on these platforms. v2: * Use nicer version check Python expression [Lluís] * Avoid pkgutil.iter_modules() [Andreas] v3: * Fix missing sys.exit() in configure check [Lluís] Stefan Hajnoczi (5): tracetool: use Python 2.4-compatible exception handling syntax tracetool: use Python 2.4-compatible __import__() arguments tracetool: avoid str.rpartition() Python 2.5 function tracetool: avoid pkgutil.iter_modules() Python 2.7 function configure: check for supported Python 2.x versions configure |7 --- scripts/tracetool.py |4 ++-- scripts/tracetool/__init__.py | 19 +++ scripts/tracetool/backend/__init__.py |8 ++-- scripts/tracetool/format/__init__.py |8 ++-- 5 files changed, 29 insertions(+), 17 deletions(-) -- 1.7.10
[Qemu-devel] [PATCH v3 1/5] tracetool: use Python 2.4-compatible exception handling syntax
The newer "except as :" syntax is not supported by Python 2.4, we need to use "except , :". Tested all trace backends with Python 2.4. Reported-by: Andreas Färber Signed-off-by: Stefan Hajnoczi Reviewed-by: Lluís Vilanova --- scripts/tracetool.py |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tracetool.py b/scripts/tracetool.py index cacfd99..c003cf6 100755 --- a/scripts/tracetool.py +++ b/scripts/tracetool.py @@ -70,7 +70,7 @@ def main(args): try: opts, args = getopt.getopt(args[1:], "", long_opts) -except getopt.GetoptError as err: +except getopt.GetoptError, err: error_opt(str(err)) check_backend = False @@ -131,7 +131,7 @@ def main(args): try: tracetool.generate(sys.stdin, arg_format, arg_backend, binary = binary, probe_prefix = probe_prefix) -except tracetool.TracetoolError as e: +except tracetool.TracetoolError, e: error_opt(str(e)) if __name__ == "__main__": -- 1.7.10
Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode
On Tue, May 1, 2012 at 14:25, Alexander Graf wrote: > > > On 01.05.2012, at 11:15, Blue Swirl wrote: > >> On Mon, Apr 30, 2012 at 11:51, Alexander Graf wrote: >>> >>> On 30.04.2012, at 12:45, Alexander Graf wrote: >>> On 22.04.2012, at 15:26, Blue Swirl wrote: > Add an explicit CPUPPCState parameter instead of relying on AREG0 > and rename op_helper.c (which only contains load and store helpers) > to mem_helper.c. Remove AREG0 swapping in > tlb_fill(). > > Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation > and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores. This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1. >>> >>> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg >>> target. It looks as if the 64-bit-guest-registers-in-32-bit-host-registers >>> code path is missing completely. >>> >>> This actually makes me less confident that this is a change we want for >>> 1.1. I'll remove the patches from the queue. >> >> Meh. It should be perfectly OK to apply all patches except the last >> one which enables the AREG0 free mode. > > Yeah, that's what I did at first, but that still didn't get me into usable > user space inside a ppc64 guest. Right now I don't have the time to track > down if it's due to your patches and if so, why. On second thought, I probably tested the set too lightly, so 'perfectly OK' was way too bold a claim. >> Also the problem with last >> patch is not in the patch itself but PPC TCG host support, which by >> the way is probably also broken for AREG0 free Sparc and Alpha, so I'd >> really like to see them in 1.1. > > I do agree on the first part. We need to make sure to test sparc and alpha > targets on unusual host platforms and fix them there. But why do we need to > also break ppc along the way? > >> There should be plenty of time to fix >> bugs in PPC TCG support during the freeze. > > Since this is a non user visible feature (in fact, looking at the emitted asm > code it'll be more of a slowdown than anything), I'd rather like to keep 1.1 > stable and get this into git right after the release split. > > It's really not going against your patches. In fact, I really do like them - > especially the cleanups. But this feature is pretty invasive and at least I > do run ppc-on-ppc tcg, so we should be able to hammer out all bugs until the > next release :). The whole AREG0 thing could also use some optimization > love... OK. > > > Alex >
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On May 1, 2012 10:37 AM, "Peter Maydell" wrote: > > On 1 May 2012 16:26, Anthony Liguori wrote: > > On 05/01/2012 10:20 AM, Peter Maydell wrote: > >> The assumption is that failure to connect is a fatal error, > >> and we can happily just assert()/hw_error()/etc. > > > > So that means that we have a bug from someone miss-typing, instead of your > > hotplug attempt failing with an error, your entire guest is destroyed. That > > doesn't sound very nice to me. > > If you're trying to hotplug a buggily implemented device then > all bets are off, really. Its never okay to kill a guest. We should fail gracefully when possible or simply avoid failing in the first place. > > Device initialization should never exit() (unless you really hit a fatal > > error like OOM). > > > > > (No, this doesn't do compile time type checking. I don't > think that's a problem particularly, or at least not enough > of one to justify not doing it this way. The object model we > have is dynamic and does things at runtime...) > >>> > >>> > >>> Correctness is more important to me than brevity. > >>> > >>> And really, we should focus on killing things like i8259_init(). > >> > >> > >> Functions like i8259_init() exist precisely because > >> QOM/qdev don't provide brevity and people trying to > >> use these devices do in fact value brevity. > > > > > > No, they exist because we aren't modeling correctly. > > Most of them are simply convenience functions that just > do "create, configure, wire up, init". (The ones that do > more than that need fixing anyway. > > i8259_init() is doing a few different things at once. > > Once you split things between init and realize, you no longer > > have long chunks of redundant code. > > ...you have redundant code scattered in multiple places instead? Redundant is the wrong word. It seems like the code ought to be reduced to a single function call but it really cannot. > > >> That's why > >> I want the standard native "connect this thing to this > >> other thing" function to be short and simple. > > > > > > With my previous proposal, it's just: > > > > s->irq_in = &b->int_out[0]; > > > > This is why I like exposing public members in structures. It's brief and > > safe. > > Obvious question: why isn't property setting done this way, > then? Surely it's equally brief and safe to say > cpu->level = def_level; > rather than > object_property_set_int(OBJECT(cpu), def->level, "level", &error); A primary design consideration for QOM was to *not* require using the generic property interface to set properties. That doesnt mean that you can/should access all properties this way. It depends on the specific property. > I don't particularly object to this sort of "public struct > vs private struct" object model, it's just not what you've > actually implemented. One step at a time. Regards, Anthony Liguori > > -- PMM
Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts
Am 01.05.2012 10:58, schrieb Alexander Graf: > On my PPC host, HOST_LONG_SIZE is not defined even after > running configure. Use the normal C way of determining the > long size instead. > > Signed-off-by: Alexander Graf > --- > thunk.h |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/thunk.h b/thunk.h > index 5be8f91..87025c3 100644 > --- a/thunk.h > +++ b/thunk.h > @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype > *type_ptr, int is_host) >defined(HOST_PARISC) || defined(HOST_SPARC64) > return 4; > #elif defined(HOST_PPC) > -return HOST_LONG_SIZE; > +return sizeof(void *); malc has committed a different fix (TARGET_ABI_BITS / 8) - thanks for fixing the build - but this PULL will now conflict. Andreas > #else > return 2; > #endif -- 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 05/14] qdev: use wrapper for qdev_get_path
Hi Gerd, Could you carefully review the USB changes here? I'm not really sure what our contract is with the guest in terms of ABI compatibility. I think it's good but it could use a second set of eyes. Regards, Anthony Liguori On 05/01/2012 01:18 PM, Anthony Liguori wrote: This makes it easier to remove it from BusInfo. Signed-off-by: Anthony Liguori --- exec.c|4 ++-- hw/qdev.c | 16 hw/qdev.h |2 ++ hw/usb/desc.c |7 +-- savevm.c | 12 ++-- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/exec.c b/exec.c index 0607c9b..e3523d2 100644 --- a/exec.c +++ b/exec.c @@ -2583,8 +2583,8 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev) assert(new_block); assert(!new_block->idstr[0]); -if (dev&& dev->parent_bus&& dev->parent_bus->info->get_dev_path) { -char *id = dev->parent_bus->info->get_dev_path(dev); +if (dev) { +char *id = qdev_get_dev_path(dev); if (id) { snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id); g_free(id); diff --git a/hw/qdev.c b/hw/qdev.c index e17a9ab..e835650 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -519,6 +519,22 @@ char* qdev_get_fw_dev_path(DeviceState *dev) return strdup(path); } +char *qdev_get_dev_path(DeviceState *dev) +{ +BusClass *bc; + +if (!dev->parent_bus) { +return NULL; +} + +bc = BUS_GET_CLASS(dev->parent_bus); +if (bc->get_dev_path) { +return bc->get_dev_path(dev); +} + +return NULL; +} + static char *qdev_get_type(Object *obj, Error **errp) { return g_strdup(object_get_typename(obj)); diff --git a/hw/qdev.h b/hw/qdev.h index ca8386a..fc3b50f 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -362,4 +362,6 @@ extern int qdev_hotplug; void qdev_add_properties(DeviceState *dev, Property *props); +char *qdev_get_dev_path(DeviceState *dev); + #endif diff --git a/hw/usb/desc.c b/hw/usb/desc.c index e8a3c6a..64352c9 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -433,11 +433,14 @@ void usb_desc_create_serial(USBDevice *dev) int index = desc->id.iSerialNumber; char serial[64]; int dst; +char *path = NULL; assert(index != 0&& desc->str[index] != NULL); dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]); -if (hcd&& hcd->parent_bus&& hcd->parent_bus->info->get_dev_path) { -char *path = hcd->parent_bus->info->get_dev_path(hcd); +if (hcd->parent_bus&& hcd->parent_bus->parent) { +path = qdev_get_dev_path(hcd->parent_bus->parent); +} +if (path) { dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path); } dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path); diff --git a/savevm.c b/savevm.c index 2d18bab..818ddfc 100644 --- a/savevm.c +++ b/savevm.c @@ -1248,8 +1248,8 @@ int register_savevm_live(DeviceState *dev, se->is_ram = 1; } -if (dev&& dev->parent_bus&& dev->parent_bus->info->get_dev_path) { -char *id = dev->parent_bus->info->get_dev_path(dev); +if (dev) { +char *id = qdev_get_dev_path(dev); if (id) { pstrcpy(se->idstr, sizeof(se->idstr), id); pstrcat(se->idstr, sizeof(se->idstr), "/"); @@ -1292,8 +1292,8 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) SaveStateEntry *se, *new_se; char id[256] = ""; -if (dev&& dev->parent_bus&& dev->parent_bus->info->get_dev_path) { -char *path = dev->parent_bus->info->get_dev_path(dev); +if (dev) { +char *path = qdev_get_dev_path(dev); if (path) { pstrcpy(id, sizeof(id), path); pstrcat(id, sizeof(id), "/"); @@ -1334,8 +1334,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id, se->alias_id = alias_id; se->no_migrate = vmsd->unmigratable; -if (dev&& dev->parent_bus&& dev->parent_bus->info->get_dev_path) { -char *id = dev->parent_bus->info->get_dev_path(dev); +if (dev) { +char *id = qdev_get_dev_path(dev); if (id) { pstrcpy(se->idstr, sizeof(se->idstr), id); pstrcat(se->idstr, sizeof(se->idstr), "/");
[Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name
This is technically a compatibility breaker. However: 1) libvirt does not rely on this (it always uses the driver name) 2) This behavior isn't actually documented anywhere (the docs just say driver). 3) I suspect there are less than three people on earth that even know this is possible (minus the people reading this message). So I think we can safely break it :-) Signed-off-by: Anthony Liguori --- hw/qdev-properties.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 98dd06a..894fa79 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -1172,8 +1172,7 @@ void qdev_prop_set_globals(DeviceState *dev) GlobalProperty *prop; QTAILQ_FOREACH(prop, &global_props, next) { -if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0 && -strcmp(qdev_get_bus_info(dev)->name, prop->driver) != 0) { +if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0) { continue; } if (qdev_prop_parse(dev, prop->property, prop->value) != 0) { -- 1.7.5.4