[Qemu-devel] [PATCH] PPC: Add VSX to hflags
We generate different code depending on whether MSR_VSX is set or clear, so it needs to be part of our hflags too which indicate whether we're still in the same translation block cache bucket. Signed-off-by: Alexander Graf ag...@suse.de --- target-ppc/helper_regs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h index a6d5e2f..c02e8da 100644 --- a/target-ppc/helper_regs.h +++ b/target-ppc/helper_regs.h @@ -56,7 +56,7 @@ static inline void hreg_compute_hflags(CPUPPCState *env) /* We 'forget' FE0 FE1: we'll never generate imprecise exceptions */ hflags_mask = (1 MSR_VR) | (1 MSR_AP) | (1 MSR_SA) | (1 MSR_PR) | (1 MSR_FP) | (1 MSR_SE) | (1 MSR_BE) | -(1 MSR_LE); +(1 MSR_LE) | (1 MSR_VSX); hflags_mask |= (1ULL MSR_CM) | (1ULL MSR_SF) | MSR_HVB; hreg_compute_mem_idx(env); env-hflags = env-msr hflags_mask; -- 1.8.1.4
Re: [Qemu-devel] [PATCH 36/39] memory: move bitmap synchronization to its own function
On 06.11.2013, at 17:22, Juan Quintela quint...@redhat.com wrote: Paolo Bonzini pbonz...@redhat.com wrote: Il 06/11/2013 14:04, Juan Quintela ha scritto: We want to have all the functions that handle directly the dirty bitmap near. We will change it later. Please move it to exec.c instead. Signed-off-by: Juan Quintela quint...@redhat.com --- include/exec/memory-physical.h | 31 +++ kvm-all.c | 27 ++- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/include/exec/memory-physical.h b/include/exec/memory-physical.h index 610c55f..72faf06 100644 --- a/include/exec/memory-physical.h +++ b/include/exec/memory-physical.h @@ -72,6 +72,37 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, xen_modified_memory(start, length); } +static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, Why le? little endian, name is already too long (cpu_physical_memory_ is too big as a preffix.) Paolo + ram_addr_t start, + ram_addr_t pages) +{ +unsigned int i, j; +unsigned long page_number, c; +hwaddr addr; +ram_addr_t ram_addr; +unsigned int len = (pages + HOST_LONG_BITS - 1) / HOST_LONG_BITS; +unsigned long hpratio = getpagesize() / TARGET_PAGE_SIZE; + +/* + * bitmap-traveling is faster than memory-traveling (for addr...) + * especially when most of the memory is not dirty. + */ +for (i = 0; i len; i++) { +if (bitmap[i] != 0) { +c = leul_to_cpu(bitmap[i]); this is the important part. KVM maintains the bitmap is Little Endian mode, and PPC? I assme needs this. Yes, there's a good chance you're running 32bit user space on a 64bit kernel on PPC. With big endian bitmaps that completely blows my mind, so I've settled for little endian bitmaps back in the day (which make a lot more sense anyway) :). +do { +j = ffsl(c) - 1; +c = ~(1ul j); +page_number = (i * HOST_LONG_BITS + j) * hpratio; +addr = page_number * TARGET_PAGE_SIZE; +ram_addr = start + addr; +cpu_physical_memory_set_dirty_range(ram_addr, +TARGET_PAGE_SIZE * hpratio); I checked, and it appears that ppc really use the hpratio thing. I hope that the bitmap optimization also works for ppc, but its architecture is weird^Winteresting O:-) Well, it's a ration between host and target page size. TARGET_PAGE_SIZE defines the smallest chunk that the MMU can possibly split things down to. Host page size however is the smallest chunk that the Linux page allocation can give us. That's a very important distinction. All server PPC hardware can use 4k page sizes. But you can configure the kernel to use 64k pages on 64bit. For older hardware, that means that we're using the full 16 HTAB group entries for a single page (16*4 = 64) which gives measurable speedups. For newer hardware we can even have real 64k pages. So while QEMU always needs to keep the guest's view of the page table world at 4k to stay compatible with what the guest is trying to do, the host kernel may have itself configured to 64k pages. This is not really PPC specific. AArch64 has a similar thing with native support for 64k page size. There are a few x86 cores with 64k page size too, but I don't think any of them run Linux :). Either way, x86 as we have it today is really a special case with TARGET_PAGE_SIZE == host_page_size. Alex
Re: [Qemu-devel] [PATCH uq/master] kvm: x86: Separately write feature control MSR on reset
Il 17/12/2013 20:05, Jan Kiszka ha scritto: If the guest is running in nested mode on system reset, clearing the feature MSR signals the kernel to leave this mode. Recent kernels processes this properly, but leave the VCPU state undefined behind. It is the job of userspace to bring it to a proper shape. Therefore, write this specific MSR first so that no state transfer gets lost. This allows to cleanly reset a guest with VMX in use. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- target-i386/kvm.c | 32 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 1188482..ec51447 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1104,6 +1104,25 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu) return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data); } +/* + * Provide a separate write service for the feature control MSR in order to + * kick the VCPU out of VMXON or even guest mode on reset. This has to be done + * before writing any other state because forcibly leaving nested mode + * invalidates the VCPU state. + */ +static int kvm_put_msr_feature_control(X86CPU *cpu) +{ +struct { +struct kvm_msrs info; +struct kvm_msr_entry entry; +} msr_data; + +kvm_msr_entry_set(msr_data.entry, MSR_IA32_FEATURE_CONTROL, + cpu-env.msr_ia32_feature_control); +msr_data.info.nmsrs = 1; +return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data); +} + static int kvm_put_msrs(X86CPU *cpu, int level) { CPUX86State *env = cpu-env; @@ -1204,10 +1223,8 @@ static int kvm_put_msrs(X86CPU *cpu, int level) if (cpu-hyperv_vapic) { kvm_msr_entry_set(msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0); } -if (has_msr_feature_control) { -kvm_msr_entry_set(msrs[n++], MSR_IA32_FEATURE_CONTROL, - env-msr_ia32_feature_control); -} +/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see + * kvm_put_msr_feature_control. */ } if (env-mcg_cap) { int i; @@ -1801,6 +1818,13 @@ int kvm_arch_put_registers(CPUState *cpu, int level) assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu)); +if (level = KVM_PUT_RESET_STATE has_msr_feature_control) { +ret = kvm_put_msr_feature_control(x86_cpu); +if (ret 0) { +return ret; +} +} + ret = kvm_getput_regs(x86_cpu, 1); if (ret 0) { return ret; Applied, thanks!
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On Wed, Dec 18, 2013 at 12:03:24AM +0100, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? I didn't have any particular list in mind beyond just protocol for now. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH -V7 3/3] target-ppc: Fix page table lookup with kvm enabled
On 07.11.2013, at 15:31, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote: From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com With kvm enabled, we store the hash page table information in the hypervisor. Use ioctl to read the htab contents. Without this we get the below error when trying to read the guest address (gdb) x/10 do_fork 0xc0098660 do_fork: Cannot access memory at address 0xc0098660 (gdb) Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com --- Changes from V6: * drop htab_fd argument and use global variable kvmppc_kern_htab instead hw/ppc/spapr.c | 1 + hw/ppc/spapr_hcall.c| 50 +++ target-ppc/kvm.c| 53 + target-ppc/kvm_ppc.h| 19 target-ppc/mmu-hash64.c | 78 - target-ppc/mmu-hash64.h | 23 ++- 6 files changed, 183 insertions(+), 41 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index d4f3502..8bf886e 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -662,6 +662,7 @@ static void spapr_reset_htab(sPAPREnvironment *spapr) if (shift 0) { /* Kernel handles htab, we don't need to allocate one */ spapr-htab_shift = shift; +kvmppc_kern_htab = true; } else { if (!spapr-htab) { /* Allocate an htab if we don't yet have one */ diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index f10ba8a..f9ea691 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -50,8 +50,9 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr, target_ulong ptel = args[3]; target_ulong page_shift = 12; target_ulong raddr; -target_ulong i; +target_ulong index; hwaddr hpte; +void *token; /* only handle 4k and 16M pages for now */ if (pteh HPTE64_V_LARGE) { @@ -94,30 +95,37 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPREnvironment *spapr, if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) { return H_PARAMETER; } + +index = 0; +hpte = pte_index * HASH_PTE_SIZE_64; if (likely((flags H_EXACT) == 0)) { pte_index = ~7ULL; -hpte = pte_index * HASH_PTE_SIZE_64; -for (i = 0; ; ++i) { -if (i == 8) { +token = ppc_hash64_start_access(cpu, pte_index); +do { +if (index == 8) { +ppc_hash64_stop_access(token); return H_PTEG_FULL; } -if ((ppc_hash64_load_hpte0(env, hpte) HPTE64_V_VALID) == 0) { +if ((ppc_hash64_load_hpte0(env, token, index) HPTE64_V_VALID) == 0) { break; } -hpte += HASH_PTE_SIZE_64; -} +} while (index++); +ppc_hash64_stop_access(token); } else { -i = 0; -hpte = pte_index * HASH_PTE_SIZE_64; -if (ppc_hash64_load_hpte0(env, hpte) HPTE64_V_VALID) { +token = ppc_hash64_start_access(cpu, pte_index); +if (ppc_hash64_load_hpte0(env, token, 0) HPTE64_V_VALID) { +ppc_hash64_stop_access(token); return H_PTEG_FULL; } +ppc_hash64_stop_access(token); } +hpte += index * HASH_PTE_SIZE_64; + ppc_hash64_store_hpte1(env, hpte, ptel); I'm not a big fan of fixing the read part, but leaving the write part broken. However I can see value in read only already, so I'm fine if the write part follows later. /* eieio(); FIXME: need some sort of barrier for smp? */ ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY); -args[0] = pte_index + i; +args[0] = pte_index + index; return H_SUCCESS; } @@ -134,16 +142,17 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex, target_ulong *vp, target_ulong *rp) { hwaddr hpte; +void *token; target_ulong v, r, rb; if ((ptex * HASH_PTE_SIZE_64) ~env-htab_mask) { return REMOVE_PARM; } -hpte = ptex * HASH_PTE_SIZE_64; - -v = ppc_hash64_load_hpte0(env, hpte); -r = ppc_hash64_load_hpte1(env, hpte); +token = ppc_hash64_start_access(ppc_env_get_cpu(env), ptex); +v = ppc_hash64_load_hpte0(env, token, 0); +r = ppc_hash64_load_hpte1(env, token, 0); +ppc_hash64_stop_access(token); if ((v HPTE64_V_VALID) == 0 || ((flags H_AVPN) (v ~0x7fULL) != avpn) || @@ -152,6 +161,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, target_ulong ptex, } *vp = v; *rp = r; +hpte = ptex * HASH_PTE_SIZE_64; ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY); rb = compute_tlbie_rb(v, r, ptex); ppc_tlb_invalidate_one(env, rb); @@ -260,16 +270,17 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPREnvironment *spapr, target_ulong pte_index = args[1];
Re: [Qemu-devel] [PATCH -V2] kvm: Add a new machine property kvm_type
On 07.11.2013, at 15:27, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote: From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com Targets like ppc64 support different typed of KVM, one which use hypervisor mode and the other which doesn't. Add a new machine property kvm_type that helps in selecting the respective ones We also add a new QEMUMachine callback get_vm_type that helps in mapping the string representation of kvm type specified. Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com --- Changes from V1: * change get_vm_type to kvm_type * change helper text hw/ppc/e500plat.c | 2 ++ hw/ppc/kvmtype.h | 18 ++ hw/ppc/mac_newworld.c | 2 ++ hw/ppc/mac_oldworld.c | 2 ++ hw/ppc/ppc440_bamboo.c | 2 ++ hw/ppc/spapr.c | 19 +++ include/hw/boards.h| 3 +++ include/hw/xen/xen.h | 3 ++- include/sysemu/kvm.h | 4 ++-- include/sysemu/qtest.h | 5 +++-- kvm-all.c | 16 +--- kvm-stub.c | 4 +++- qtest.c| 2 +- vl.c | 17 +++-- xen-all.c | 2 +- xen-stub.c | 2 +- 16 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 hw/ppc/kvmtype.h diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c index 2e964b2..4be4b24 100644 --- a/hw/ppc/e500plat.c +++ b/hw/ppc/e500plat.c @@ -17,6 +17,7 @@ #include hw/pci/pci.h #include hw/ppc/openpic.h #include kvm_ppc.h +#include kvmtype.h static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt) { @@ -51,6 +52,7 @@ static QEMUMachine e500plat_machine = { .desc = generic paravirt e500 platform, .init = e500plat_init, .max_cpus = 32, +.kvm_type = pr_kvm_type, What about mpc8544ds? Also, e500plat can definitely support HV (e500mc+) and PR (e500v2) mode. So you probably want the same check here as you have in spapr. Unfortunately the patch doesn't apply anymore. Would you care to rebase it? I think it's also more targeted towards the kvm queue rather than the ppc queue, so I'd either like an ack from Paolo or would defer to his tree for inclusion :). Alex
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? Orit Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram
On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote: Hi, We need to change the way we reserve the mmconfig space though. Currently it is marked reserved in the e820 table. Having that overlap with the _CRS region makes windows quite unhappy, we tried that recently. Yes this also contradicts the spec, see below. My laptop has the mmconfig space declared as LPC ressource: Device (LPC) { Name (_ADR, 0x001F) // _ADR: Address Name (_S3D, 0x03) // _S3D: S3 Device State Name (RID, 0x00) Device (SIO) { Name (_HID, EisaId (PNP0C02)) Name (_UID, 0x00) // _UID: Unique ID Name (SCRS, ResourceTemplate () [ ... ] Memory32Fixed (ReadWrite, 0xF800, // Address Base 0x0400, // Address Length ) [ ... ] Method (_CRS, 0, NotSerialized) [ ... return SCRS, with updates applied in some cases ... ] When doing it this way we can simply make the PCI0._CRS cover the whole end-of-ram - ioapic-base range, simliar to piix, and we are pretty free to place the mmconfig xbar anywhere in that area. The spec says: (under \_SB) in a node with a _HID of EISAID (PNP0C02), So this is what my laptop does. and the resources in this case should not be claimed in the root PCI bus’s _CRS. My laptop has them in the root bus _CRS though: [0.124634] PCI: MMCONFIG at [mem 0xf800-0xfbff] reserved in ACPI motherboard resources [0.139391] pci_bus :00: root bus resource [mem 0xdfa0-0xfebf] The resources can optionally be returned in Int15 E820 or EFIGetMemoryMap as reserved memory but must always be reported through ACPI as a motherboard resource. So we can do both e820 and motherboard ressource. Good, that hopefully simplifies the transition. My reading of the above is that this can be an LPC resource but claiming this as the root's _CRS isn't ok then. I read the specs the same way, but my laptop does something different. Guess that needs quite some testing to figure which works best ... I merged your patch but split it: q35 is separate and piix is separate. Would you like me to drop the q35 part then? If you are fine with q35 having only 2G lowmem keep it. It's safe. We can sort the mmconfig setup afterwards, then check if (and how) we'll transition to 3G lowmem. Maybe we simply don't after all, with the world moving to 64bit it doesn't matter that much whenever memory is mapped above or below 4g. And for old 32bit guests there is always the option to stick with piix which continues to offers up to 3.5G lowmem. cheers, Gerd I'll think it over, I will keep the patch around but won't merge to Anthony meanwhile. -- MST
Re: [Qemu-devel] [PATCH 07/38] memory: make cpu_physical_memory_is_dirty return bool
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 7 ++- include/exec/memory-internal.h | 8 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/exec.c b/exec.c index 95bcf70..a1fc280 100644 --- a/exec.c +++ b/exec.c @@ -1484,11 +1484,8 @@ found: static void notdirty_mem_write(void *opaque, hwaddr ram_addr, uint64_t val, unsigned size) { -int dirty_flags; -dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr); if (!cpu_physical_memory_get_dirty_flag(ram_addr, CODE_DIRTY_FLAG)) { tb_invalidate_phys_page_fast(ram_addr, size); -dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr); } switch (size) { case 1: @@ -1503,8 +1500,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, default: abort(); } -dirty_flags |= (0xff ~CODE_DIRTY_FLAG); -cpu_physical_memory_set_dirty_flags(ram_addr, dirty_flags); +cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG); /* we remove the notdirty callback only if the code has been flushed */ if (cpu_physical_memory_is_dirty(ram_addr)) { diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 136198c..0b25c3f 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -56,9 +56,13 @@ static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, } /* read dirty bit (return 0 or 1) */ -static inline int cpu_physical_memory_is_dirty(ram_addr_t addr) +static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr) { -return cpu_physical_memory_get_dirty_flags(addr) == 0xff; +bool vga = cpu_physical_memory_get_dirty_flag(addr, VGA_DIRTY_FLAG); +bool code = cpu_physical_memory_get_dirty_flag(addr, CODE_DIRTY_FLAG); +bool migration = +cpu_physical_memory_get_dirty_flag(addr, MIGRATION_DIRTY_FLAG); +return vga code migration; } static inline int cpu_physical_memory_get_dirty(ram_addr_t start, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 08/38] memory: all users of cpu_physical_memory_get_dirty used only one flag
On 12/17/2013 05:25 PM, Juan Quintela wrote: So cpu_physical_memory_get_dirty_flags is not needed anymore Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 0b25c3f..53cfe83 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -44,11 +44,6 @@ void qemu_ram_free_from_ptr(ram_addr_t addr); #define CODE_DIRTY_FLAG 0x02 #define MIGRATION_DIRTY_FLAG 0x08 -static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr) -{ -return ram_list.phys_dirty[addr TARGET_PAGE_BITS]; -} - static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, int dirty_flag) { @@ -67,7 +62,7 @@ static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr) static inline int cpu_physical_memory_get_dirty(ram_addr_t start, ram_addr_t length, -int dirty_flags) +int dirty_flag) { int ret = 0; ram_addr_t addr, end; @@ -75,7 +70,7 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start, end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -ret |= cpu_physical_memory_get_dirty_flags(addr) dirty_flags; +ret |= cpu_physical_memory_get_dirty_flag(addr, dirty_flag); } return ret; } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 09/38] memory: set single dirty flags when possible
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 7 --- include/exec/memory-internal.h | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/exec.c b/exec.c index a1fc280..6981f73 100644 --- a/exec.c +++ b/exec.c @@ -1911,7 +1911,8 @@ static void invalidate_and_set_dirty(hwaddr addr, /* invalidate code */ tb_invalidate_phys_page_range(addr, addr + length, 0); /* set dirty bit */ -cpu_physical_memory_set_dirty_flags(addr, (0xff ~CODE_DIRTY_FLAG)); +cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG); } xen_modified_memory(addr, length); } @@ -2493,8 +2494,8 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val) /* invalidate code */ tb_invalidate_phys_page_range(addr1, addr1 + 4, 0); /* set dirty bit */ -cpu_physical_memory_set_dirty_flags( -addr1, (0xff ~CODE_DIRTY_FLAG)); +cpu_physical_memory_set_dirty_flag(addr1, MIGRATION_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr1, VGA_DIRTY_FLAG); } } } diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 53cfe83..9f4ad69 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -89,7 +89,9 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) { -cpu_physical_memory_set_dirty_flags(addr, 0xff); +cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr, CODE_DIRTY_FLAG); } static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 10/38] memory: cpu_physical_memory_set_dirty_range() always dirty all flags
On 12/17/2013 05:25 PM, Juan Quintela wrote: So remove the flag argument and do it directly. After this change, there is nothing else using cpu_physical_memory_set_dirty_flags() so remove it. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 2 +- include/exec/memory-internal.h | 11 ++- memory.c | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/exec.c b/exec.c index 6981f73..45fdb84 100644 --- a/exec.c +++ b/exec.c @@ -1274,7 +1274,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, last_ram_offset() TARGET_PAGE_BITS); memset(ram_list.phys_dirty + (new_block-offset TARGET_PAGE_BITS), 0, size TARGET_PAGE_BITS); -cpu_physical_memory_set_dirty_range(new_block-offset, size, 0xff); +cpu_physical_memory_set_dirty_range(new_block-offset, size); qemu_ram_setup_dump(new_block-host, size); qemu_madvise(new_block-host, size, QEMU_MADV_HUGEPAGE); diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 9f4ad69..681d63b 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -75,12 +75,6 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start, return ret; } -static inline void cpu_physical_memory_set_dirty_flags(ram_addr_t addr, - int dirty_flags) -{ -ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= dirty_flags; -} - static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, int dirty_flag) { @@ -103,15 +97,14 @@ static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr, } static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, - ram_addr_t length, - int dirty_flags) + ram_addr_t length) { ram_addr_t addr, end; end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -cpu_physical_memory_set_dirty_flags(addr, dirty_flags); +cpu_physical_memory_set_dirty(addr); } xen_modified_memory(addr, length); } diff --git a/memory.c b/memory.c index e497f99..fb52e1c 100644 --- a/memory.c +++ b/memory.c @@ -1182,7 +1182,7 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size) { assert(mr-terminates); -cpu_physical_memory_set_dirty_range(mr-ram_addr + addr, size, -1); +cpu_physical_memory_set_dirty_range(mr-ram_addr + addr, size); } bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 11/38] memory: cpu_physical_memory_mask_dirty_range() always clears a single flag
On 12/17/2013 05:25 PM, Juan Quintela wrote: Document it Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- cputlb.c | 4 ++-- exec.c | 19 ++- include/exec/memory-internal.h | 40 ++-- include/exec/memory.h | 3 --- memory.c | 10 -- 5 files changed, 34 insertions(+), 42 deletions(-) diff --git a/cputlb.c b/cputlb.c index 72452e5..dfd747a 100644 --- a/cputlb.c +++ b/cputlb.c @@ -129,7 +129,7 @@ void tlb_protect_code(ram_addr_t ram_addr) { cpu_physical_memory_reset_dirty(ram_addr, ram_addr + TARGET_PAGE_SIZE, -CODE_DIRTY_FLAG); +DIRTY_MEMORY_CODE); } /* update the TLB so that writes in physical page 'phys_addr' are no longer @@ -137,7 +137,7 @@ void tlb_protect_code(ram_addr_t ram_addr) void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr, target_ulong vaddr) { -cpu_physical_memory_set_dirty_flag(ram_addr, CODE_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_CODE); } static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe) diff --git a/exec.c b/exec.c index 45fdb84..9996da2 100644 --- a/exec.c +++ b/exec.c @@ -737,7 +737,7 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t end, /* Note: start and end must be within the same ram block. */ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, - int dirty_flags) + unsigned client) { uintptr_t length; @@ -747,7 +747,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, length = end - start; if (length == 0) return; -cpu_physical_memory_mask_dirty_range(start, length, dirty_flags); +cpu_physical_memory_mask_dirty_range(start, length, client); if (tcg_enabled()) { tlb_reset_dirty_range_all(start, end, length); @@ -1484,7 +1484,7 @@ found: static void notdirty_mem_write(void *opaque, hwaddr ram_addr, uint64_t val, unsigned size) { -if (!cpu_physical_memory_get_dirty_flag(ram_addr, CODE_DIRTY_FLAG)) { +if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) { tb_invalidate_phys_page_fast(ram_addr, size); } switch (size) { @@ -1500,8 +1500,8 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, default: abort(); } -cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG); -cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_MIGRATION); +cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_VGA); /* we remove the notdirty callback only if the code has been flushed */ if (cpu_physical_memory_is_dirty(ram_addr)) { @@ -1911,8 +1911,8 @@ static void invalidate_and_set_dirty(hwaddr addr, /* invalidate code */ tb_invalidate_phys_page_range(addr, addr + length, 0); /* set dirty bit */ -cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG); -cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA); +cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); } xen_modified_memory(addr, length); } @@ -2494,8 +2494,9 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val) /* invalidate code */ tb_invalidate_phys_page_range(addr1, addr1 + 4, 0); /* set dirty bit */ -cpu_physical_memory_set_dirty_flag(addr1, MIGRATION_DIRTY_FLAG); -cpu_physical_memory_set_dirty_flag(addr1, VGA_DIRTY_FLAG); +cpu_physical_memory_set_dirty_flag(addr1, + DIRTY_MEMORY_MIGRATION); +cpu_physical_memory_set_dirty_flag(addr1, DIRTY_MEMORY_VGA); } } } diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 681d63b..b58010f 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -40,29 +40,25 @@ void *qemu_get_ram_ptr(ram_addr_t addr); void qemu_ram_free(ram_addr_t addr); void qemu_ram_free_from_ptr(ram_addr_t addr); -#define VGA_DIRTY_FLAG 0x01 -#define CODE_DIRTY_FLAG 0x02 -#define MIGRATION_DIRTY_FLAG 0x08 - static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, - int dirty_flag) + unsigned client) { -return ram_list.phys_dirty[addr TARGET_PAGE_BITS]
Re: [Qemu-devel] [PATCH 12/38] memory: use bit 2 for migration
On 12/17/2013 05:25 PM, Juan Quintela wrote: For historical reasons it was bit 3. Once there, create a constant to know the number of clients. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index b8e76f4..d5e9d58 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -35,7 +35,8 @@ typedef struct MemoryRegionMmio MemoryRegionMmio; #define DIRTY_MEMORY_VGA 0 #define DIRTY_MEMORY_CODE 1 -#define DIRTY_MEMORY_MIGRATION 3 +#define DIRTY_MEMORY_MIGRATION 2 +#define DIRTY_MEMORY_NUM 3/* num of dirty bits */ struct MemoryRegionMmio { CPUReadMemoryFunc *read[3]; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 13/38] memory: make sure that client is always inside range
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 4 1 file changed, 4 insertions(+) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index b58010f..d09d6d8 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -43,6 +43,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr); static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, unsigned client) { +assert(client DIRTY_MEMORY_NUM); return ram_list.phys_dirty[addr TARGET_PAGE_BITS] (1 client); } @@ -74,6 +75,7 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start, static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, unsigned client) { +assert(client DIRTY_MEMORY_NUM); ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= (1 client); } @@ -89,6 +91,8 @@ static inline int cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, { int mask = ~(1 client); +assert(client DIRTY_MEMORY_NUM); + return ram_list.phys_dirty[addr TARGET_PAGE_BITS] = mask; } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 15/38] memory: cpu_physical_memory_clear_dirty_flag() result is never used
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index d09d6d8..666490c 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -86,14 +86,14 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); } -static inline int cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, +static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, unsigned client) { int mask = ~(1 client); assert(client DIRTY_MEMORY_NUM); -return ram_list.phys_dirty[addr TARGET_PAGE_BITS] = mask; +ram_list.phys_dirty[addr TARGET_PAGE_BITS] = mask; } static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 14/38] memory: only resize dirty bitmap when memory size increases
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index 9996da2..bed5c07 100644 --- a/exec.c +++ b/exec.c @@ -1210,6 +1210,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, MemoryRegion *mr) { RAMBlock *block, *new_block; +ram_addr_t old_ram_size, new_ram_size; + +old_ram_size = last_ram_offset() TARGET_PAGE_BITS; size = TARGET_PAGE_ALIGN(size); new_block = g_malloc0(sizeof(*new_block)); @@ -1270,10 +1273,13 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, ram_list.version++; qemu_mutex_unlock_ramlist(); -ram_list.phys_dirty = g_realloc(ram_list.phys_dirty, - last_ram_offset() TARGET_PAGE_BITS); -memset(ram_list.phys_dirty + (new_block-offset TARGET_PAGE_BITS), +new_ram_size = last_ram_offset() TARGET_PAGE_BITS; + +if (new_ram_size old_ram_size) { +ram_list.phys_dirty = g_realloc(ram_list.phys_dirty, new_ram_size); +memset(ram_list.phys_dirty + (new_block-offset TARGET_PAGE_BITS), 0, size TARGET_PAGE_BITS); +} cpu_physical_memory_set_dirty_range(new_block-offset, size); qemu_ram_setup_dump(new_block-host, size); Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On Wed, Dec 18, 2013 at 12:00:03PM +0200, Orit Wasserman wrote: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? Since this is bypassing the client kernel FS I/O layer question around support of things like O_SYNC/O_DIRECT are not applicable. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 16/38] bitmap: Add bitmap_zero_extend operation
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/qemu/bitmap.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h index afdd257..1babd5d 100644 --- a/include/qemu/bitmap.h +++ b/include/qemu/bitmap.h @@ -220,4 +220,13 @@ unsigned long bitmap_find_next_zero_area(unsigned long *map, unsigned long nr, unsigned long align_mask); +static inline unsigned long *bitmap_zero_extend(unsigned long *old, +long old_nbits, long new_nbits) +{ +long new_len = BITS_TO_LONGS(new_nbits) * sizeof(unsigned long); +unsigned long *new = g_realloc(old, new_len); +bitmap_clear(new, old_nbits, new_nbits - old_nbits); +return new; +} + #endif /* BITMAP_H */ Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 17/38] memory: split dirty bitmap into three
On 12/17/2013 05:25 PM, Juan Quintela wrote: After all the previous patches, spliting the bitmap gets direct. Note: For some reason, I have to move DIRTY_MEMORY_* definitions to the beginning of memory.h to make compilation work. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 9 ++--- include/exec/cpu-all.h | 3 ++- include/exec/memory-internal.h | 9 +++-- include/exec/memory.h | 10 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/exec.c b/exec.c index bed5c07..ad9866c 100644 --- a/exec.c +++ b/exec.c @@ -1276,9 +1276,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, new_ram_size = last_ram_offset() TARGET_PAGE_BITS; if (new_ram_size old_ram_size) { -ram_list.phys_dirty = g_realloc(ram_list.phys_dirty, new_ram_size); -memset(ram_list.phys_dirty + (new_block-offset TARGET_PAGE_BITS), - 0, size TARGET_PAGE_BITS); +int i; +for (i = 0; i DIRTY_MEMORY_NUM; i++) { +ram_list.dirty_memory[i] = +bitmap_zero_extend(ram_list.dirty_memory[i], + old_ram_size, new_ram_size); + } } cpu_physical_memory_set_dirty_range(new_block-offset, size); diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index b6998f0..4cb4b4a 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -21,6 +21,7 @@ #include qemu-common.h #include exec/cpu-common.h +#include exec/memory.h #include qemu/thread.h #include qom/cpu.h @@ -459,7 +460,7 @@ typedef struct RAMBlock { typedef struct RAMList { QemuMutex mutex; /* Protected by the iothread lock. */ -uint8_t *phys_dirty; +unsigned long *dirty_memory[DIRTY_MEMORY_NUM]; RAMBlock *mru_block; /* Protected by the ramlist lock. */ QTAILQ_HEAD(, RAMBlock) blocks; diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 666490c..6fb1b64 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -44,7 +44,7 @@ static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, unsigned client) { assert(client DIRTY_MEMORY_NUM); -return ram_list.phys_dirty[addr TARGET_PAGE_BITS] (1 client); +return test_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } /* read dirty bit (return 0 or 1) */ @@ -76,7 +76,7 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, unsigned client) { assert(client DIRTY_MEMORY_NUM); -ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= (1 client); +set_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) @@ -89,11 +89,8 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, unsigned client) { -int mask = ~(1 client); - assert(client DIRTY_MEMORY_NUM); - -ram_list.phys_dirty[addr TARGET_PAGE_BITS] = mask; +clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, diff --git a/include/exec/memory.h b/include/exec/memory.h index d5e9d58..296d6ab 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -16,6 +16,11 @@ #ifndef CONFIG_USER_ONLY +#define DIRTY_MEMORY_VGA 0 +#define DIRTY_MEMORY_CODE 1 +#define DIRTY_MEMORY_MIGRATION 2 +#define DIRTY_MEMORY_NUM 3/* num of dirty bits */ + #include stdint.h #include stdbool.h #include qemu-common.h @@ -33,11 +38,6 @@ typedef struct MemoryRegionOps MemoryRegionOps; typedef struct MemoryRegionMmio MemoryRegionMmio; -#define DIRTY_MEMORY_VGA 0 -#define DIRTY_MEMORY_CODE 1 -#define DIRTY_MEMORY_MIGRATION 2 -#define DIRTY_MEMORY_NUM 3/* num of dirty bits */ - struct MemoryRegionMmio { CPUReadMemoryFunc *read[3]; CPUWriteMemoryFunc *write[3]; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On 12/18/2013 12:18 PM, Daniel P. Berrange wrote: On Wed, Dec 18, 2013 at 12:00:03PM +0200, Orit Wasserman wrote: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? Since this is bypassing the client kernel FS I/O layer question around support of things like O_SYNC/O_DIRECT are not applicable. so no live migration support? Daniel
Re: [Qemu-devel] [PATCH 18/38] memory: unfold cpu_physical_memory_clear_dirty_flag() in its only user
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 6fb1b64..2f6610a 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -86,13 +86,6 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); } -static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, - unsigned client) -{ -assert(client DIRTY_MEMORY_NUM); -clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); -} - static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, ram_addr_t length) { @@ -112,10 +105,11 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start, { ram_addr_t addr, end; +assert(client DIRTY_MEMORY_NUM); end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -cpu_physical_memory_clear_dirty_flag(addr, client); +clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 19/38] memory: unfold cpu_physical_memory_set_dirty() in its only user
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 2f6610a..9d34434 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -79,13 +79,6 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, set_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } -static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) -{ -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA); -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); -} - static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, ram_addr_t length) { @@ -94,7 +87,9 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -cpu_physical_memory_set_dirty(addr); +cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); +cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA); +cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); } xen_modified_memory(addr, length); } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 20/38] memory: unfold cpu_physical_memory_set_dirty_flag()
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 9d34434..b99617a 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -87,9 +87,12 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA); -cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); +set_bit(addr TARGET_PAGE_BITS, +ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]); +set_bit(addr TARGET_PAGE_BITS, +ram_list.dirty_memory[DIRTY_MEMORY_VGA]); +set_bit(addr TARGET_PAGE_BITS, +ram_list.dirty_memory[DIRTY_MEMORY_CODE]); } xen_modified_memory(addr, length); } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 21/38] memory: make cpu_physical_memory_get_dirty() the main function
On 12/17/2013 05:25 PM, Juan Quintela wrote: And make cpu_physical_memory_get_dirty_flag() to use it. It used to be the other way around. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index b99617a..edca8a8 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -40,11 +40,28 @@ void *qemu_get_ram_ptr(ram_addr_t addr); void qemu_ram_free(ram_addr_t addr); void qemu_ram_free_from_ptr(ram_addr_t addr); +static inline int cpu_physical_memory_get_dirty(ram_addr_t start, +ram_addr_t length, +unsigned client) +{ +int ret = 0; +ram_addr_t addr, end; + +assert(client DIRTY_MEMORY_NUM); + +end = TARGET_PAGE_ALIGN(start + length); +start = TARGET_PAGE_MASK; +for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { +ret |= test_bit(addr TARGET_PAGE_BITS, +ram_list.dirty_memory[client]); +} +return ret; +} + static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, unsigned client) { -assert(client DIRTY_MEMORY_NUM); -return test_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); +return cpu_physical_memory_get_dirty(addr, 1, client); } /* read dirty bit (return 0 or 1) */ @@ -57,21 +74,6 @@ static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr) return vga code migration; } -static inline int cpu_physical_memory_get_dirty(ram_addr_t start, -ram_addr_t length, -unsigned client) -{ -int ret = 0; -ram_addr_t addr, end; - -end = TARGET_PAGE_ALIGN(start + length); -start = TARGET_PAGE_MASK; -for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -ret |= cpu_physical_memory_get_dirty_flag(addr, client); -} -return ret; -} - static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, unsigned client) { Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 22/38] memory: cpu_physical_memory_get_dirty() is used as returning a bool
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index edca8a8..fa28fc6 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -40,11 +40,10 @@ void *qemu_get_ram_ptr(ram_addr_t addr); void qemu_ram_free(ram_addr_t addr); void qemu_ram_free_from_ptr(ram_addr_t addr); -static inline int cpu_physical_memory_get_dirty(ram_addr_t start, -ram_addr_t length, -unsigned client) +static inline bool cpu_physical_memory_get_dirty(ram_addr_t start, + ram_addr_t length, + unsigned client) { -int ret = 0; ram_addr_t addr, end; assert(client DIRTY_MEMORY_NUM); @@ -52,10 +51,12 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t start, end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -ret |= test_bit(addr TARGET_PAGE_BITS, -ram_list.dirty_memory[client]); +if (test_bit(addr TARGET_PAGE_BITS, + ram_list.dirty_memory[client])) { +return true; +} } -return ret; +return false; } static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 18/38] memory: unfold cpu_physical_memory_clear_dirty_flag() in its only user
On 12/17/2013 05:25 PM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 6fb1b64..2f6610a 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -86,13 +86,6 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t addr) cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE); } -static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr, - unsigned client) -{ -assert(client DIRTY_MEMORY_NUM); -clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); -} - static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, ram_addr_t length) { @@ -112,10 +105,11 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start, { ram_addr_t addr, end; +assert(client DIRTY_MEMORY_NUM); end = TARGET_PAGE_ALIGN(start + length); start = TARGET_PAGE_MASK; for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -cpu_physical_memory_clear_dirty_flag(addr, client); +clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
Il 17/12/2013 20:38, Anthony Liguori ha scritto: On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 00:26, Anthony Liguori ha scritto: Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. Can you explain what you mean? The question is whether hotpluggable as a property applies to all devices or not. But hotplug is strictly a bus level concept. It's a sequence of events that correspond to what happens when you add a new device to a bus after power on. Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Igor's interface applies just as well to the case of plugging a device at startup; I think separating the two makes little sense. And once you have cold-plug and hot-plug in qdev core, it makes sense to add unplug as well. Also because we already have surprise removal in qdev core (that's unparent) and we have some kind of unplug request support (device_del/dc-unplug). One possibility that remains is to put cold/hot-plug in a BusDevice class rather than in the core qdev: Device BusDevice-- can be cold/hot-plugged but this adds more complication. For example, the same CPU can be hotpluggable or not depending on the board model, should the superclass be Device or BusDevice. And if we ever have multi-CPU targets, with the core CPU not hotpluggable and additional hotpluggable ones (e.g. for GPUs) what would be the superclass of the common CPU superclass? The question is whether there can be code sharing without touching the base class. You could certainly have a HotpluggableBusState and then a HotpluggableDeviceState. Interfaces would be another option too. Interfaces are fine, but the question is who finds them and calls them. In this case, the discovery mechanism is a link property, and the calling mechanism is an explicit hook in the realized property. If we had aspect-oriented programming, we would be marking join points instead of writing if (dev-foo) bar(dev-foo) conditionals. But the idea is the same. The general concern is about polluting widely used base classes. It's better if we can avoid adding things to DeviceState and Object whenever possible. I agree. At the same time we should make base classes as small as possible, but not smaller than that. Paolo
Re: [Qemu-devel] [PATCH 23/38] memory: s/mask/clear/ cpu_physical_memory_mask_dirty_range
On 12/17/2013 05:25 PM, Juan Quintela wrote: Now all functions use the same wording that bitops/bitmap operations Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 2 +- include/exec/memory-internal.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exec.c b/exec.c index ad9866c..6bef3e5 100644 --- a/exec.c +++ b/exec.c @@ -747,7 +747,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, length = end - start; if (length == 0) return; -cpu_physical_memory_mask_dirty_range(start, length, client); +cpu_physical_memory_clear_dirty_range(start, length, client); if (tcg_enabled()) { tlb_reset_dirty_range_all(start, end, length); diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index fa28fc6..c04a92a 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -100,9 +100,9 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, xen_modified_memory(addr, length); } -static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start, -ram_addr_t length, -unsigned client) +static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, + ram_addr_t length, + unsigned client) { ram_addr_t addr, end; Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
Il 18/12/2013 11:24, Orit Wasserman ha scritto: For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? Since this is bypassing the client kernel FS I/O layer question around support of things like O_SYNC/O_DIRECT are not applicable. so no live migration support? No, live migration just works. O_SYNC is not used anymore in QEMU, we just issue a flush after every write. And all protocol drivers except files/block devices _always_ bypass the kernel page cache (libiscsi, NBD, ceph, and now libnfs) so they are always behaving as if they had O_DIRECT. For these protocols, cache=writeback and cache=none are entirely the same. Paolo
Re: [Qemu-devel] [qemu-kvm RHEL7 PATCH] Add support statement to -help output
Il 04/12/2013 19:42, Eduardo Habkost ha scritto: Bugzilla: 972773 Brew scratch build: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=6676272 Add support statement to -help output, reporting direct qemu-kvm usage as unsupported by Red Hat, and advising users to use libvirt instead. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- vl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/vl.c b/vl.c index 9b1738b..fdc8c65 100644 --- a/vl.c +++ b/vl.c @@ -1995,9 +1995,17 @@ static void version(void) printf(QEMU emulator version QEMU_VERSION QEMU_PKGVERSION , Copyright (c) 2003-2008 Fabrice Bellard\n); } +static void print_rh_warning(void) +{ +printf(\nWARNING: Direct use of qemu-kvm from the command line is not supported by Red Hat.\n + WARNING: Use libvirt as the stable management interface.\n + WARNING: Some command line options listed here may not be available in future releases.\n\n); +} + static void help(int exitcode) { version(); +print_rh_warning(); printf(usage: %s [options] [disk_image]\n\n 'disk_image' is a raw hard disk image for IDE hard disk 0\n\n, error_get_progname()); @@ -2012,6 +2020,7 @@ static void help(int exitcode) \n When using -nographic, press 'ctrl-a h' to get some help.\n); +print_rh_warning(); exit(exitcode); } Wow, no whitelist anymore?!?!? ACK Paolo
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
Am 18.12.2013 um 11:00 schrieb Orit Wasserman owass...@redhat.com: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? the async you see in the libnfs calls refer to the async API. bdrv_flush will not return before the nfs server completes the request. Peter Orit Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On 12/18/2013 01:11 PM, Peter Lieven wrote: Am 18.12.2013 um 11:00 schrieb Orit Wasserman owass...@redhat.com: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? the async you see in the libnfs calls refer to the async API. bdrv_flush will not return before the nfs server completes the request. That great! Thanks, Orit Peter Orit Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen: build on ARM
On Tue, 2013-12-17 at 17:31 +, Stefano Stabellini wrote: Collection of fixes to build QEMU with Xen support on ARM: - use xenstore_read_fe_uint64 to retrieve the page-ref (xenfb); - use xen_pfn_t instead of unsigned long in xenfb; - unsigned long/xenpfn_t in xen_remove_from_physmap; - add __arm__ and __aarch64__ cases in xen-mapcache.c. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- hw/display/xenfb.c | 17 + xen-all.c |2 +- xen-mapcache.c |4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index f0333a0..cbc2901 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -93,10 +93,11 @@ struct XenFB { static int common_bind(struct common *c) { -int mfn; +uint64_t mfn; The commit message says you are switching to use xen_pfn_t in this file... Somewhere along the line this will need to be cast to a xen_pfn_t, which would potentially involve a loss of the upper bits if xen_pfn_t is only 32-bits. You might want an if (mfn != (xen_pfn_t)mfn) style check somewhere. Ian.
[Qemu-devel] Enabling Travis for the QEMU Mirror?
Hi, Now we have a .travis.yml merged into master can we enable Travis to run in the QEMU mirror on github (https://github.com/qemu/qemu)? I'm happy to talk who ever controls that repo through the process but it's fairly simple. From the commit: This adds a build matrix definition for travis-ci.org continuous integration service. It is usable on any public repository hosted on GitHub. Once you have created an account signed into Travis you can enable it on selected projects via travis-ci.org/profile. Alternatively you can configure the service hooks on GitHub via the repository Settings tab,then Service Hooks and selecting Travis. Basically you just need to go to http://travis-ci.org and click the Sign in with GitHub link and your up and running. It's super easy. -- Alex Bennée QEMU/KVM Hacker for Linaro
Re: [Qemu-devel] Enabling Travis for the QEMU Mirror?
On 18 December 2013 11:32, Alex Bennée alex.ben...@linaro.org wrote: Now we have a .travis.yml merged into master can we enable Travis to run in the QEMU mirror on github (https://github.com/qemu/qemu)? I'm happy to talk who ever controls that repo through the process That would be Anthony, I think. but it's fairly simple. From the commit: This adds a build matrix definition for travis-ci.org continuous integration service. It is usable on any public repository hosted on GitHub. Once you have created an account signed into Travis you can enable it on selected projects via travis-ci.org/profile. Alternatively you can configure the service hooks on GitHub via the repository Settings tab,then Service Hooks and selecting Travis. Basically you just need to go to http://travis-ci.org and click the Sign in with GitHub link and your up and running. It's super easy. So this would get us I guess automatic build tests and a page somewhere with current status, right? We might want to talk about automated email-on-build-failure at some later point but running it without that to start seems a good first step. Do you have a link to a results page for what this looks like? (IIRC you've enabled it for your private repo). thanks -- PMM
Re: [Qemu-devel] [PATCH 24/38] memory: use find_next_bit() to find dirty bits
On 12/17/2013 05:26 PM, Juan Quintela wrote: This operation is way faster than doing it bit by bit. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index c04a92a..b017c2e 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -44,19 +44,15 @@ static inline bool cpu_physical_memory_get_dirty(ram_addr_t start, ram_addr_t length, unsigned client) { -ram_addr_t addr, end; +unsigned long end, page, next; assert(client DIRTY_MEMORY_NUM); -end = TARGET_PAGE_ALIGN(start + length); -start = TARGET_PAGE_MASK; -for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -if (test_bit(addr TARGET_PAGE_BITS, - ram_list.dirty_memory[client])) { -return true; -} -} -return false; +end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; +page = start TARGET_PAGE_BITS; +next = find_next_bit(ram_list.dirty_memory[client], end, page); + +return next end; } static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 25/38] memory: cpu_physical_memory_set_dirty_range() now uses bitmap operations
On 12/17/2013 05:26 PM, Juan Quintela wrote: We were setting a range of bits, so use bitmap_set(). Note: xen has always been wrong, and should have used start instead of addr from the beginning. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index b017c2e..4906cdf 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -81,19 +81,14 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, ram_addr_t length) { -ram_addr_t addr, end; +unsigned long end, page; -end = TARGET_PAGE_ALIGN(start + length); -start = TARGET_PAGE_MASK; -for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -set_bit(addr TARGET_PAGE_BITS, -ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]); -set_bit(addr TARGET_PAGE_BITS, -ram_list.dirty_memory[DIRTY_MEMORY_VGA]); -set_bit(addr TARGET_PAGE_BITS, -ram_list.dirty_memory[DIRTY_MEMORY_CODE]); -} -xen_modified_memory(addr, length); +end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; +page = start TARGET_PAGE_BITS; +bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page); +bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page); +bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page); +xen_modified_memory(start, length); } static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 27/38] memory: s/dirty/clean/ in cpu_physical_memory_is_dirty()
On 12/17/2013 05:26 PM, Juan Quintela wrote: All uses except one really want the other meaning. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- cputlb.c | 3 ++- exec.c | 6 +++--- include/exec/memory-internal.h | 5 ++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cputlb.c b/cputlb.c index dfd747a..911d764 100644 --- a/cputlb.c +++ b/cputlb.c @@ -299,7 +299,8 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr, /* Write access calls the I/O callback. */ te-addr_write = address | TLB_MMIO; } else if (memory_region_is_ram(section-mr) -!cpu_physical_memory_is_dirty(section-mr-ram_addr + xlat)) { +cpu_physical_memory_is_clean(section-mr-ram_addr + + xlat)) { te-addr_write = address | TLB_NOTDIRTY; } else { te-addr_write = address; diff --git a/exec.c b/exec.c index 6bef3e5..a2f89eb 100644 --- a/exec.c +++ b/exec.c @@ -1513,7 +1513,7 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_VGA); /* we remove the notdirty callback only if the code has been flushed */ -if (cpu_physical_memory_is_dirty(ram_addr)) { +if (!cpu_physical_memory_is_clean(ram_addr)) { CPUArchState *env = current_cpu-env_ptr; tlb_set_dirty(env, env-mem_io_vaddr); } @@ -1916,7 +1916,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, static void invalidate_and_set_dirty(hwaddr addr, hwaddr length) { -if (!cpu_physical_memory_is_dirty(addr)) { +if (cpu_physical_memory_is_clean(addr)) { /* invalidate code */ tb_invalidate_phys_page_range(addr, addr + length, 0); /* set dirty bit */ @@ -2499,7 +2499,7 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val) stl_p(ptr, val); if (unlikely(in_migration)) { -if (!cpu_physical_memory_is_dirty(addr1)) { +if (cpu_physical_memory_is_clean(addr1)) { /* invalidate code */ tb_invalidate_phys_page_range(addr1, addr1 + 4, 0); /* set dirty bit */ diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index e2f55ea..771b23f 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -61,14 +61,13 @@ static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, return cpu_physical_memory_get_dirty(addr, 1, client); } -/* read dirty bit (return 0 or 1) */ -static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr) +static inline bool cpu_physical_memory_is_clean(ram_addr_t addr) { bool vga = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_VGA); bool code = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_CODE); bool migration = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); -return vga code migration; +return !(vga code migration); } static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 28/38] memory: make cpu_physical_memory_reset_dirty() take a length parameter
On 12/17/2013 05:26 PM, Juan Quintela wrote: We have an end parameter in all the callers, and this make it coherent with the rest of cpu_physical_memory_* functions, that also take a length parameter. Once here, move the start/end calculation to tlb_reset_dirty_range_all() as we don't need it here anymore. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- cputlb.c | 3 +-- exec.c | 19 --- include/exec/memory-internal.h | 2 +- memory.c | 8 ++-- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/cputlb.c b/cputlb.c index 911d764..a5805e1 100644 --- a/cputlb.c +++ b/cputlb.c @@ -127,8 +127,7 @@ void tlb_flush_page(CPUArchState *env, target_ulong addr) can be detected */ void tlb_protect_code(ram_addr_t ram_addr) { -cpu_physical_memory_reset_dirty(ram_addr, -ram_addr + TARGET_PAGE_SIZE, +cpu_physical_memory_reset_dirty(ram_addr, TARGET_PAGE_SIZE, DIRTY_MEMORY_CODE); } diff --git a/exec.c b/exec.c index a2f89eb..ba5989c 100644 --- a/exec.c +++ b/exec.c @@ -723,11 +723,14 @@ found: return block; } -static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t end, - uintptr_t length) +static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length) { -RAMBlock *block; ram_addr_t start1; +RAMBlock *block; +ram_addr_t end; + +end = TARGET_PAGE_ALIGN(start + length); +start = TARGET_PAGE_MASK; block = qemu_get_ram_block(start); assert(block == qemu_get_ram_block(end - 1)); @@ -736,21 +739,15 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t end, } /* Note: start and end must be within the same ram block. */ -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, +void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, unsigned client) { -uintptr_t length; - -start = TARGET_PAGE_MASK; -end = TARGET_PAGE_ALIGN(end); - -length = end - start; if (length == 0) return; cpu_physical_memory_clear_dirty_range(start, length, client); if (tcg_enabled()) { -tlb_reset_dirty_range_all(start, end, length); +tlb_reset_dirty_range_all(start, length); } } diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 771b23f..cb2249f 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -102,7 +102,7 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, bitmap_clear(ram_list.dirty_memory[client], page, end - page); } -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, +void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, unsigned client); #endif diff --git a/memory.c b/memory.c index a490cbd..c010296 100644 --- a/memory.c +++ b/memory.c @@ -1191,9 +1191,7 @@ bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr, assert(mr-terminates); ret = cpu_physical_memory_get_dirty(mr-ram_addr + addr, size, client); if (ret) { -cpu_physical_memory_reset_dirty(mr-ram_addr + addr, -mr-ram_addr + addr + size, -client); +cpu_physical_memory_reset_dirty(mr-ram_addr + addr, size, client); } return ret; } @@ -1239,9 +1237,7 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { assert(mr-terminates); -cpu_physical_memory_reset_dirty(mr-ram_addr + addr, -mr-ram_addr + addr + size, -client); +cpu_physical_memory_reset_dirty(mr-ram_addr + addr, size, client); } void *memory_region_get_ram_ptr(MemoryRegion *mr) Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 29/38] memory: cpu_physical_memory_set_dirty_tracking() should return void
On 12/17/2013 05:26 PM, Juan Quintela wrote: Result was always 0, and not used anywhere. Once there, use bool type for the parameter. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- exec.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/exec.c b/exec.c index ba5989c..d165fd8 100644 --- a/exec.c +++ b/exec.c @@ -56,7 +56,7 @@ //#define DEBUG_SUBPAGE #if !defined(CONFIG_USER_ONLY) -static int in_migration; +static bool in_migration; RAMList ram_list = { .blocks = QTAILQ_HEAD_INITIALIZER(ram_list.blocks) }; @@ -751,11 +751,9 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, } } -static int cpu_physical_memory_set_dirty_tracking(int enable) +static void cpu_physical_memory_set_dirty_tracking(bool enable) { -int ret = 0; in_migration = enable; -return ret; } hwaddr memory_region_section_get_iotlb(CPUArchState *env, @@ -1797,12 +1795,12 @@ static void tcg_commit(MemoryListener *listener) static void core_log_global_start(MemoryListener *listener) { -cpu_physical_memory_set_dirty_tracking(1); +cpu_physical_memory_set_dirty_tracking(true); } static void core_log_global_stop(MemoryListener *listener) { -cpu_physical_memory_set_dirty_tracking(0); +cpu_physical_memory_set_dirty_tracking(false); } static MemoryListener core_memory_listener = { Reviewed-by: Orit Wasserman owass...@redhat.com
[Qemu-devel] sniffing traffic between virtual machines
Hello Friends, Thanks for your hints; they really helped us! We are trying to monitor the traffic (network packets etc) between VMs in KVM. We succeeded to get the address of the system call table (see http://syprog.blogspot.co.il/2011/10/hijack-linux-system-calls-part-iii.html) and intercept the system calls going through the kernel. In such a way we see ALL system calls (including those which were not initiated from within VMs). How can we filter out the system calls not related to VMs ? What is your opinion regarding our approach ? Best Regards, Mark, Martin, Alex On Mon 14 Oct 11:12 2013 Stefan Hajnoczi wrote: On Sat, Oct 12, 2013 at 05:45:52PM +0300, Alexander Binun wrote: The qemu used by me is the one installed using apt-get install qemu. The executable is in /usr/bin. The KVM driver is the one supplied with Ubuntu 13.04. The version of qemu is 1.4.0 (after running qemu --version I get the message --- QEMU emulator version 1.4.0 (Debian 1.4.0+dfsg-1expubuntu4), Copyright (c) 2003-2008 Fabrice Bellard You mean I should use the build-from-sources qemu (getting the sources from git://git.qemu-project.org/qemu.git) ? Should I then compile from sources and mount the KVM ? In that case it sounds like everything is coming from Ubuntu 13.04 and should work together. Sorry, I don't know about Ubuntu 13.04. Perhaps there is already a solution if you search the Ubuntu bug tracker. Stefan
Re: [Qemu-devel] [PATCH 30/38] memory: split cpu_physical_memory_* functions to its own include
On 12/17/2013 05:26 PM, Juan Quintela wrote: All the functions that use ram_addr_t should be here. Signed-off-by: Juan Quintela quint...@redhat.com --- cputlb.c | 1 + exec.c | 1 + include/exec/memory-internal.h | 76 include/exec/ram_addr.h| 98 ++ memory.c | 1 + 5 files changed, 101 insertions(+), 76 deletions(-) create mode 100644 include/exec/ram_addr.h diff --git a/cputlb.c b/cputlb.c index a5805e1..a1e8421 100644 --- a/cputlb.c +++ b/cputlb.c @@ -26,6 +26,7 @@ #include exec/cputlb.h #include exec/memory-internal.h +#include exec/ram_addr.h //#define DEBUG_TLB //#define DEBUG_TLB_CHECK diff --git a/exec.c b/exec.c index d165fd8..be421e5 100644 --- a/exec.c +++ b/exec.c @@ -50,6 +50,7 @@ #include translate-all.h #include exec/memory-internal.h +#include exec/ram_addr.h #include qemu/range.h diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index cb2249f..25c43c0 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -20,9 +20,6 @@ #define MEMORY_INTERNAL_H #ifndef CONFIG_USER_ONLY -#include hw/xen/xen.h - - typedef struct AddressSpaceDispatch AddressSpaceDispatch; void address_space_init_dispatch(AddressSpace *as); @@ -33,78 +30,5 @@ extern const MemoryRegionOps unassigned_mem_ops; bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, unsigned size, bool is_write); -ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, - MemoryRegion *mr); -ram_addr_t qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr); -void *qemu_get_ram_ptr(ram_addr_t addr); -void qemu_ram_free(ram_addr_t addr); -void qemu_ram_free_from_ptr(ram_addr_t addr); - -static inline bool cpu_physical_memory_get_dirty(ram_addr_t start, - ram_addr_t length, - unsigned client) -{ -unsigned long end, page, next; - -assert(client DIRTY_MEMORY_NUM); - -end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; -page = start TARGET_PAGE_BITS; -next = find_next_bit(ram_list.dirty_memory[client], end, page); - -return next end; -} - -static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr, - unsigned client) -{ -return cpu_physical_memory_get_dirty(addr, 1, client); -} - -static inline bool cpu_physical_memory_is_clean(ram_addr_t addr) -{ -bool vga = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_VGA); -bool code = cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_CODE); -bool migration = -cpu_physical_memory_get_dirty_flag(addr, DIRTY_MEMORY_MIGRATION); -return !(vga code migration); -} - -static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, - unsigned client) -{ -assert(client DIRTY_MEMORY_NUM); -set_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); -} - -static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, - ram_addr_t length) -{ -unsigned long end, page; - -end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; -page = start TARGET_PAGE_BITS; -bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page); -bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page); -bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page); -xen_modified_memory(start, length); -} - -static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, - ram_addr_t length, - unsigned client) -{ -unsigned long end, page; - -assert(client DIRTY_MEMORY_NUM); -end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; -page = start TARGET_PAGE_BITS; -bitmap_clear(ram_list.dirty_memory[client], page, end - page); -} - -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, - unsigned client); - #endif - #endif diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h new file mode 100644 index 000..db977fb --- /dev/null +++ b/include/exec/ram_addr.h @@ -0,0 +1,98 @@ +/* + * Declarations for cpu physical memory functions + * + * Copyright 2011 Red Hat, Inc. and/or its affiliates + * + * Authors: + * Avi Kivity a...@redhat.com + * + * 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 header is for use by exec.c and memory.c ONLY. Do not include it. + * The functions
Re: [Qemu-devel] Enabling Travis for the QEMU Mirror?
peter.mayd...@linaro.org writes: On 18 December 2013 11:32, Alex Bennée alex.ben...@linaro.org wrote: Now we have a .travis.yml merged into master can we enable Travis to run in the QEMU mirror on github (https://github.com/qemu/qemu)? I'm happy to talk who ever controls that repo through the process That would be Anthony, I think. but it's fairly simple. From the commit: This adds a build matrix definition for travis-ci.org continuous integration service. It is usable on any public repository hosted on GitHub. Once you have created an account signed into Travis you can enable it on selected projects via travis-ci.org/profile. Alternatively you can configure the service hooks on GitHub via the repository Settings tab,then Service Hooks and selecting Travis. Basically you just need to go to http://travis-ci.org and click the Sign in with GitHub link and your up and running. It's super easy. So this would get us I guess automatic build tests and a page somewhere with current status, right? We might want to talk about automated email-on-build-failure at some later point but running it without that to start seems a good first step. Yes, the rest like notifications or IRC bots can be configured. We might want to convert the top level README to a README.md and insert a fancy icon link like I have in my other repos: https://github.com/stsquad/emacs_chrome#hacking Do you have a link to a results page for what this looks like? (IIRC you've enabled it for your private repo). Sure have a gander at: https://travis-ci.org/stsquad/qemu and https://travis-ci.org/stsquad/qemu/branches etc... thanks -- PMM -- Alex Bennée QEMU/KVM Hacker for Linaro
Re: [Qemu-devel] [PATCH 26/38] memory: cpu_physical_memory_clear_dirty_range() now uses bitmap operations
On 12/17/2013 05:26 PM, Juan Quintela wrote: We were clearing a range of bits, so use bitmap_clear(). Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/exec/memory-internal.h | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 4906cdf..e2f55ea 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -95,14 +95,12 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, ram_addr_t length, unsigned client) { -ram_addr_t addr, end; +unsigned long end, page; assert(client DIRTY_MEMORY_NUM); -end = TARGET_PAGE_ALIGN(start + length); -start = TARGET_PAGE_MASK; -for (addr = start; addr end; addr += TARGET_PAGE_SIZE) { -clear_bit(addr TARGET_PAGE_BITS, ram_list.dirty_memory[client]); -} +end = TARGET_PAGE_ALIGN(start + length) TARGET_PAGE_BITS; +page = start TARGET_PAGE_BITS; +bitmap_clear(ram_list.dirty_memory[client], page, end - page); } void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end, Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH 31/38] memory: unfold memory_region_test_and_clear()
On 12/17/2013 05:26 PM, Juan Quintela wrote: We are going to update the bitmap directly Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/arch_init.c b/arch_init.c index e0acbc5..0e8c8b5 100644 --- a/arch_init.c +++ b/arch_init.c @@ -48,6 +48,7 @@ #include qmp-commands.h #include trace.h #include exec/cpu-all.h +#include exec/ram_addr.h #include hw/acpi/acpi.h #ifdef DEBUG_ARCH_INIT @@ -400,9 +401,12 @@ static void migration_bitmap_sync(void) QTAILQ_FOREACH(block, ram_list.blocks, next) { for (addr = 0; addr block-length; addr += TARGET_PAGE_SIZE) { -if (memory_region_test_and_clear_dirty(block-mr, - addr, TARGET_PAGE_SIZE, - DIRTY_MEMORY_MIGRATION)) { +if (cpu_physical_memory_get_dirty(block-mr-ram_addr + addr, + TARGET_PAGE_SIZE, + DIRTY_MEMORY_MIGRATION)) { +cpu_physical_memory_reset_dirty(block-mr-ram_addr + addr, +TARGET_PAGE_SIZE, +DIRTY_MEMORY_MIGRATION); migration_bitmap_set_dirty(block-mr, addr); } } Reviewed-by: Orit Wasserman owass...@redhat.com
Re: [Qemu-devel] [PATCH] PPC: fix PCI configuration space MemoryRegions for grackle/uninorth
On 08.11.2013, at 23:18, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: On 08/11/13 03:20, Alexander Graf wrote: On 11.10.2013, at 12:53, Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk wrote: OpenBIOS prior to SVN r1225 had a horrible bug when accessing PCI configuration space for PPC Mac architectures - instead of writing the PCI configuration data value to the data register address, it would instead write it to the data register address plus the PCI configuration address. For this reason, the MemoryRegions for the PCI data register for grackle/uninorth were extremely large in order to accomodate the entire PCI configuration space being accessed during OpenBIOS PCI bus enumeration. Now that the OpenBIOS images have been updated, reduce the MemoryRegion sizes down to a single 32-bit register as intended. Signed-off-by: Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk CC: Hervé Poussineauhpous...@reactos.org CC: Andreas Färberafaer...@suse.de CC: Alexander Grafag...@suse.de With this patch applied, mac99 emulation seems to break: http://award.ath.cx/results/288-alex/x86/kvm.qemu-git-tcg.ppc-debian.mac99-G4.etch.e1000.reboot/debug/serial-vm1.log Alex Hi Alex, Thanks for the heads-up - with the information above I was able to reproduce this fairly easily. I had look at some of the uninorth drivers, and while it's not particularly apparent from Linux that the PCI configuration data is accessed via MMIO rather than ioport access, FreeBSD seems to suggest that this is the case: http://code.google.com/p/freebsd-head/source/browse/sys/powerpc/powermac/uninorth.c?spec=svnc6989e24706228678e454517dad4ad465a36e556r=c6989e24706228678e454517dad4ad465a36e556#274. The key is that the QEMU uninorth host bridge contains a hack to allow PCI configuration mechanism #1 as used by OpenBIOS to work at all (see unin_get_config_reg() in hw/pci-host/uninorth.c) which is why I didn't notice it in my OpenBIOS boot tests; and in fact, the name of the uninorth PCI configuration data MemoryRegions have a -data rather than a -idx suffix is also a big clue. Hence please find a revised version of the patch which is unaltered for grackle, and only changes the MemoryRegion size for the PCI configuration address register for uninorth so that the PCI configuration data space is still accessible using MMIO. This resolves the issue for me, so if you're satisifed that it works for you then I'll post a revised version to the list. Hrm. Are you 100% sure this correct? This UniNorth is a real headache. The closest thing to a spec for it is the U4 spec which is generations ahead: http://www.datasheetarchive.com/dl/Datasheets-SW3/DSASW0048084.pdf On that at page 109 you can see that you do indeed have a range of registers and a few fancy modes that can even be used to directly access config space registers without the usual index/data cycle. Ben, do you have any more insight into how the original Uninorth worked? Alex
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen: build on ARM
On Wed, 18 Dec 2013, Ian Campbell wrote: On Tue, 2013-12-17 at 17:31 +, Stefano Stabellini wrote: Collection of fixes to build QEMU with Xen support on ARM: - use xenstore_read_fe_uint64 to retrieve the page-ref (xenfb); - use xen_pfn_t instead of unsigned long in xenfb; - unsigned long/xenpfn_t in xen_remove_from_physmap; - add __arm__ and __aarch64__ cases in xen-mapcache.c. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- hw/display/xenfb.c | 17 + xen-all.c |2 +- xen-mapcache.c |4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index f0333a0..cbc2901 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -93,10 +93,11 @@ struct XenFB { static int common_bind(struct common *c) { -int mfn; +uint64_t mfn; The commit message says you are switching to use xen_pfn_t in this file... Somewhere along the line this will need to be cast to a xen_pfn_t, which would potentially involve a loss of the upper bits if xen_pfn_t is only 32-bits. You might want an if (mfn != (xen_pfn_t)mfn) style check somewhere. Good idea, I'll add an assert.
Re: [Qemu-devel] [PATCH v2 0/5] Monitor commands for object-add/del
On Tue, 17 Dec 2013 12:25:59 +0100 Paolo Bonzini pbonz...@redhat.com wrote: These allow hotplugging (and hot-unplugging without leaking an object) virtio-rng devices. They can also be used for memory hotplug. v1-v2: fix mistyped underscores in qapi-schema.json Paolo Bonzini (5): rng: initialize file descriptor to -1 qom: fix leak for objects created with -object qom: catch errors in object_property_add_child monitor: add object-add (QMP) and object_add (HMP) command monitor: add object-del (QMP) and object_del (HMP) command backends/rng-random.c | 4 +-- hmp-commands.hx | 28 ++ hmp.c | 67 ++ hmp.h | 2 ++ include/monitor/monitor.h | 3 ++ include/qapi/visitor.h| 3 +- include/qemu/typedefs.h | 2 ++ qapi-schema.json | 34 ++ qmp-commands.hx | 51 qmp.c | 74 +++ qom/object.c | 9 -- vl.c | 3 +- 12 files changed, 273 insertions(+), 7 deletions(-) Reviewed-By: Igor Mammedov imamm...@redhat.com
Re: [Qemu-devel] [PATCH 0/8] s390 sigp, chsc and diag bugfixes/cleanups
On 17.12.2013, at 14:22, Jens Freimann jf...@linux.vnet.ibm.com wrote: Alex, this is mostly bugfixes and cleanup patches, except for Patch 4/8 which implements a SIGP order code for cpu start. Thanks, applied all to s390-next. Alex
Re: [Qemu-devel] [PATCH V7 0/6] qcow2: rollback the modification on fail in snapshot creation
On Wed, Dec 18, 2013 at 10:13:40AM +0800, Wenchao Xia wrote: Hello, any comments? I hope to have a new year gift It's in the queue, I hope to review it this week.
[Qemu-devel] [PULL 0/8] s390 patch queue 2013-12-18
Hi Blue / Aurelien / Anthony, This is my current patch queue for s390. Please pull. Alex The following changes since commit f46e720a82ccdf1a521cf459448f3f96ed895d43: Laszlo Ersek (1): qemu_opts_parse(): always check return value are available in the git repository at: git://github.com/agraf/qemu.git s390-for-upstream Cornelia Huck (1): s390x/kvm: Fix diagnose handling. Thomas Huth (7): s390x/kvm: Removed duplicated SIGP defines s390x/kvm: Removed s390_store_status stub s390x/kvm: Fix coding style in handle_sigp() s390x/kvm: Implemented SIGP START s390x/kvm: Simplified the calculation of the SIGP order code s390x/kvm: Fixed condition code for unknown SIGP orders s390x/ioinst: CHSC has to set a condition code target-s390x/cpu.h|3 + target-s390x/ioinst.c |1 + target-s390x/kvm.c| 98 +++-- 3 files changed, 50 insertions(+), 52 deletions(-)
[Qemu-devel] [PULL 1/8] s390x/kvm: Fix diagnose handling.
From: Cornelia Huck cornelia.h...@de.ibm.com The instruction intercept handler for diagnose used only the displacement when trying to calculate the function code. This is only correct for base 0, however; we need to perform a complete base/displacement address calculation and use bits 48-63 as the function code. Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/cpu.h |3 +++ target-s390x/kvm.c | 19 +-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index a2c077b..68b5ab7 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -352,6 +352,9 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb) return addr; } +/* Base/displacement are at the same locations. */ +#define decode_basedisp_rs decode_basedisp_s + void s390x_tod_timer(void *opaque); void s390x_cpu_timer(void *opaque); diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 02ac4ba..b00a661 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -562,11 +562,19 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run) handle_diag_308(cpu-env, r1, r3); } -static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code) +#define DIAG_KVM_CODE_MASK 0x + +static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb) { int r = 0; - -switch (ipb_code) { +uint16_t func_code; + +/* + * For any diagnose call we support, bits 48-63 of the resulting + * address specify the function code; the remainder is ignored. + */ +func_code = decode_basedisp_rs(cpu-env, ipb) DIAG_KVM_CODE_MASK; +switch (func_code) { case DIAG_IPL: kvm_handle_diag_308(cpu, run); break; @@ -577,7 +585,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code) sleep(10); break; default: -DPRINTF(KVM: unknown DIAG: 0x%x\n, ipb_code); +DPRINTF(KVM: unknown DIAG: 0x%x\n, func_code); r = -1; break; } @@ -684,7 +692,6 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run) { unsigned int ipa0 = (run-s390_sieic.ipa 0xff00); uint8_t ipa1 = run-s390_sieic.ipa 0x00ff; -int ipb_code = (run-s390_sieic.ipb 0x0fff) 16; int r = -1; DPRINTF(handle_instruction 0x%x 0x%x\n, @@ -696,7 +703,7 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run) r = handle_priv(cpu, run, ipa0 8, ipa1); break; case IPA0_DIAG: -r = handle_diag(cpu, run, ipb_code); +r = handle_diag(cpu, run, run-s390_sieic.ipb); break; case IPA0_SIGP: r = handle_sigp(cpu, run, ipa1); -- 1.6.0.2
[Qemu-devel] [PULL 4/8] s390x/kvm: Fix coding style in handle_sigp()
From: Thomas Huth th...@linux.vnet.ibm.com To make scripts/checkpatch.pl happy for the following patches, the coding style in handle_sigp() has to be fixed first. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Acked-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c | 24 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 5b243b4..8c54134 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -642,18 +642,18 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) } switch (order_code) { -case SIGP_RESTART: -r = kvm_s390_cpu_restart(target_cpu); -break; -case SIGP_SET_ARCH: -/* make the caller panic */ -return -1; -case SIGP_INITIAL_CPU_RESET: -r = s390_cpu_initial_reset(target_cpu); -break; -default: -fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code); -break; +case SIGP_RESTART: +r = kvm_s390_cpu_restart(target_cpu); +break; +case SIGP_SET_ARCH: +/* make the caller panic */ +return -1; +case SIGP_INITIAL_CPU_RESET: +r = s390_cpu_initial_reset(target_cpu); +break; +default: +fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code); +break; } out: -- 1.6.0.2
[Qemu-devel] [PULL 2/8] s390x/kvm: Removed duplicated SIGP defines
From: Thomas Huth th...@linux.vnet.ibm.com The SIGP order defines are also available in cpu.h, so there is no need to re-define them in kvm.c. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c |5 - 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index b00a661..52d93a7 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -82,11 +82,6 @@ #define ICPT_CPU_STOP 0x28 #define ICPT_IO 0x40 -#define SIGP_RESTART0x06 -#define SIGP_INITIAL_CPU_RESET 0x0b -#define SIGP_STORE_STATUS_ADDR 0x0e -#define SIGP_SET_ARCH 0x12 - const KVMCapabilityInfo kvm_arch_required_capabilities[] = { KVM_CAP_LAST_INFO }; -- 1.6.0.2
[Qemu-devel] [PULL 3/8] s390x/kvm: Removed s390_store_status stub
From: Thomas Huth th...@linux.vnet.ibm.com The SIGP order STORE STATUS AT ADDRESS will be handled in kernel space, so we do not need the stub in QEMU anymore. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c | 22 -- 1 files changed, 0 insertions(+), 22 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 52d93a7..5b243b4 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -597,13 +597,6 @@ int kvm_s390_cpu_restart(S390CPU *cpu) return 0; } -static int s390_store_status(CPUS390XState *env, uint32_t parameter) -{ -/* XXX */ -fprintf(stderr, XXX SIGP store status\n); -return -1; -} - static int s390_cpu_initial_reset(S390CPU *cpu) { CPUState *cs = CPU(cpu); @@ -629,12 +622,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) { CPUS390XState *env = cpu-env; uint8_t order_code; -uint32_t parameter; uint16_t cpu_addr; -uint8_t t; int r = -1; S390CPU *target_cpu; -CPUS390XState *target_env; cpu_synchronize_state(CPU(cpu)); @@ -645,28 +635,16 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) } order_code += (run-s390_sieic.ipb 0x0fff) 16; -/* get parameters */ -t = (ipa1 0xf0) 4; -if (!(t % 2)) { -t++; -} - -parameter = env-regs[t] 0x7e00; cpu_addr = env-regs[ipa1 0x0f]; - target_cpu = s390_cpu_addr2state(cpu_addr); if (target_cpu == NULL) { goto out; } -target_env = target_cpu-env; switch (order_code) { case SIGP_RESTART: r = kvm_s390_cpu_restart(target_cpu); break; -case SIGP_STORE_STATUS_ADDR: -r = s390_store_status(target_env, parameter); -break; case SIGP_SET_ARCH: /* make the caller panic */ return -1; -- 1.6.0.2
[Qemu-devel] [PULL 5/8] s390x/kvm: Implemented SIGP START
From: Thomas Huth th...@linux.vnet.ibm.com This patch adds the missing START order to the SIGP instruction handler. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 8c54134..fcc159f 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb) return r; } +static int kvm_s390_cpu_start(S390CPU *cpu) +{ +s390_add_running_cpu(cpu); +qemu_cpu_kick(CPU(cpu)); +DPRINTF(DONE: KVM cpu start: %p\n, cpu-env); +return 0; +} + int kvm_s390_cpu_restart(S390CPU *cpu) { kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0); @@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) } switch (order_code) { +case SIGP_START: +r = kvm_s390_cpu_start(target_cpu); +break; case SIGP_RESTART: r = kvm_s390_cpu_restart(target_cpu); break; -- 1.6.0.2
[Qemu-devel] [PULL 7/8] s390x/kvm: Fixed condition code for unknown SIGP orders
From: Thomas Huth th...@linux.vnet.ibm.com If SIGP is called with an unknown order code, it has to return CC1 instead of CC3 and set the invalid order bit in the return status. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c | 17 +++-- 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 0bf3d1f..f7b7726 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -633,8 +633,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) CPUS390XState *env = cpu-env; uint8_t order_code; uint16_t cpu_addr; -int r = -1; S390CPU *target_cpu; +uint64_t *statusreg = env-regs[ipa1 4]; +int cc; cpu_synchronize_state(CPU(cpu)); @@ -644,29 +645,33 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) cpu_addr = env-regs[ipa1 0x0f]; target_cpu = s390_cpu_addr2state(cpu_addr); if (target_cpu == NULL) { +cc = 3;/* not operational */ goto out; } switch (order_code) { case SIGP_START: -r = kvm_s390_cpu_start(target_cpu); +cc = kvm_s390_cpu_start(target_cpu); break; case SIGP_RESTART: -r = kvm_s390_cpu_restart(target_cpu); +cc = kvm_s390_cpu_restart(target_cpu); break; case SIGP_SET_ARCH: /* make the caller panic */ return -1; case SIGP_INITIAL_CPU_RESET: -r = s390_cpu_initial_reset(target_cpu); +cc = s390_cpu_initial_reset(target_cpu); break; default: -fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code); +DPRINTF(KVM: unknown SIGP: 0x%x\n, order_code); +*statusreg = 0xUL; +*statusreg |= SIGP_STAT_INVALID_ORDER; +cc = 1; /* status stored */ break; } out: -setcc(cpu, r ? 3 : 0); +setcc(cpu, cc); return 0; } -- 1.6.0.2
[Qemu-devel] [PULL 8/8] s390x/ioinst: CHSC has to set a condition code
From: Thomas Huth th...@linux.vnet.ibm.com I missed to set the CC in the CHSC instruction when I refactored the CC setting in the IO instructions with the following commit: 5d9bf1c07c1369ab3506fc82cc65a10f4415d867 s390/ioinst: Moved the CC setting to the IO instruction handlers This patch now restores the correct behaviour of CHSC by setting the condition code 0 at the end of the instruction. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/ioinst.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c index 8d6363d..b8a6486 100644 --- a/target-s390x/ioinst.c +++ b/target-s390x/ioinst.c @@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb) break; } +setcc(cpu, 0);/* Command execution complete */ out: s390_cpu_physical_memory_unmap(env, req, map_size, 1); } -- 1.6.0.2
[Qemu-devel] [PULL 6/8] s390x/kvm: Simplified the calculation of the SIGP order code
From: Thomas Huth th...@linux.vnet.ibm.com We've already got a helper function for calculating the base/displacement of RS formatted instructions, so we can get rid of the manual calculation of the SIGP order code. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Alexander Graf ag...@suse.de --- target-s390x/kvm.c |8 +++- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index fcc159f..0bf3d1f 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -626,6 +626,8 @@ static int s390_cpu_initial_reset(S390CPU *cpu) return 0; } +#define SIGP_ORDER_MASK 0x00ff + static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) { CPUS390XState *env = cpu-env; @@ -637,11 +639,7 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) cpu_synchronize_state(CPU(cpu)); /* get order code */ -order_code = run-s390_sieic.ipb 28; -if (order_code 0) { -order_code = env-regs[order_code]; -} -order_code += (run-s390_sieic.ipb 0x0fff) 16; +order_code = decode_basedisp_rs(env, run-s390_sieic.ipb) SIGP_ORDER_MASK; cpu_addr = env-regs[ipa1 0x0f]; target_cpu = s390_cpu_addr2state(cpu_addr); -- 1.6.0.2
Re: [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
On 11.11.2013, at 09:16, peter.crosthwa...@xilinx.com wrote: From: Peter Crosthwaite peter.crosthwa...@xilinx.com There are a mix of usages of the qemu_fdt_* API calls, some which wish to assert and abort QEMU on failure and some of which wish to do their own error handling. The latter in more correct, but the former is in the majority and more pragmatic. However the asserting clients are usually doing large numbers fdt ops and only want to assert if any one of them breaks. So the cleanest compromising solution is: 1. Require an Error ** to be passes to all fdt ops. 2. And perform no action if its already non-null (error condition). 3. If no Error ** is given report errors to stderr Step one allows clients to do their own error handling if they wish too, which is the most flexible: Error *err = NULL; fdt_foo( ... , err); if (err) { /* handle error my special way */ } Step two allows you to do a large number of fdt ops then check them all only the once at the end: Error *err = NULL; fdt_foo( ... , err); fdt_bar( ... , err); fdt_baz( ... , err); if (err) { /* First encountered error will be in err */ } Step 3, handles the common case where the error is not an issue, but just needs to make noise at the user (via stderr). This seems common for bootloader code that sets chosen and initrd props etc (rather than machine description). All error reporting is stylistically udpated to use Error ** instead of integer return codes and no more exit(1) on failures. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Thanks a lot. I've applied the first two patches to ppc-next with some adjustments to also cover hw/arm/virt.c. But this patch is too much work to rebase to current master :). Please repost a version that also covers virt.c. I'd also like to see an Ack from Peter at least. Alex
Re: [Qemu-devel] [PATCH v4 resend] rdma: rename 'x-rdma' = 'rdma'
On 12/17/2013 10:43 PM, mrhi...@linux.vnet.ibm.com wrote: From: Michael R. Hines mrhi...@us.ibm.com As far as we can tell, all known bugs have been fixed: 1. Parallel migrations are working 2. IPv6 migration is working 3. virt-test is working +++ b/qapi-schema.json @@ -671,10 +671,9 @@ # This feature allows us to minimize migration traffic for certain work # loads, by sending compressed difference of the pages # -# @x-rdma-pin-all: Controls whether or not the entire VM memory footprint is +# @rdma-pin-all: Controls whether or not the entire VM memory footprint is # mlock()'d on demand or all at once. Refer to docs/rdma.txt for usage. -# Disabled by default. Experimental: may (or may not) be renamed after -# further testing is complete. (since 1.6) +# Disabled by default. (since 1.7) Alas, we missed 1.7. This should now read 'since 2.0'. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [qemu-kvm PATCH] block: vhdx - improve error message, and .bdrv_check implementation
On Tue, Dec 17, 2013 at 05:33:37AM -0500, Jeff Cody wrote: If there is a dirty log file to be replayed in a VHDX image, it is replayed in .vhdx_open(). However, if the file is opened read-only, then a somewhat cryptic error message results. This adds a more helpful error message for the user. If an image file contains a log to be replayed, and is opened read-only, the user is instructed to run 'qemu-img check -r all' on the image file. Running qemu-img check -r all will cause the image file to be opened r/w, which will replay the log file. If a log file replay is detected, this is flagged, and bdrv_check will increase the corruptions_fixed count for the image. Signed-off-by: Jeff Cody jc...@redhat.com --- block/vhdx-log.c | 12 +++- block/vhdx.c | 22 -- block/vhdx.h | 5 - 3 files changed, 35 insertions(+), 4 deletions(-) Merged to be replayed typo fix when applying the patch. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH] target-ppc: remove MMUCFG SPR from POWER7/8 class
On 18.11.2013, at 09:29, Alexey Kardashevskiy a...@ozlabs.ru wrote: PowerISA 2.06/2.07 put MMUCFG SPR to E (embedded) category so remove it from POWER7/8 class as it is S (server) category. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru POWER7 inherited this from the other 64bit cores. Does any of those actually implement MMUCFG? It's definitely category E in the 2.04 ISA already. Please check all supported CPU types' book iv's and remove MMUCFG (and maybe other SPRs if you can spot them) if it's not really implemented on hardware. Alex
Re: [Qemu-devel] [PATCH V4] char: restore read callback on a reattached (hotplug) chardev
On 18/12/2013 07:35, Amit Shah wrote: As far as I could tell the pty backend doesn't suffer from this issue. That's why I didn't change anything there. pty_chr_update_read_handler() calls pty_chr_state(), which calls remove_fd_in_watch(). Yes, but the pty_chr_update_read_handler() isn't called on device removal so remove_fd_in_watch() is not executed. However I did find that there was a problem in the pty backend that I missed and I'll post an updated patch. Thanks, Gal.
[Qemu-devel] [PATCH V5] char: restore read callback on a reattached (hotplug) chardev
Fix a bug that was introduced in commit 386a5a1e. A removal of a device set the chr handlers to NULL. However when the device is plugged back, its read callback is not restored so data can't be transferred from the host to the guest (e.g. via the virtio-serial port). https://bugzilla.redhat.com/show_bug.cgi?id=1027181 Signed-off-by: Gal Hammer gham...@redhat.com --- qemu-char.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) V5: - remove_fd_in_watch in fd_chr_update_read_handler as well. - fix pty backend. V4: - Same as V3, but this time done right. V3: - fix a typo in comment. - move the revision history after the signed-off-by tag. V2: - do not call chr_update_read_handler on device removal. - add asserts to verify chr_update_read_handler is not called with an assigned fd_in_tag to prevent fd leaks. - update fd and udp backends' chr_update_read_handler function so it won't remove fd_in to prevent a double release. diff --git a/qemu-char.c b/qemu-char.c index 418dc69..019c178 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -213,7 +213,7 @@ void qemu_chr_add_handlers(CharDriverState *s, s-chr_read = fd_read; s-chr_event = fd_event; s-handler_opaque = opaque; -if (s-chr_update_read_handler) +if (fe_open s-chr_update_read_handler) s-chr_update_read_handler(s); if (!s-explicit_fe_open) { @@ -870,7 +870,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr) { FDCharDriver *s = chr-opaque; -remove_fd_in_watch(chr); +assert(!chr-fd_in_tag); if (s-fd_in) { chr-fd_in_tag = io_add_watch_poll(s-fd_in, fd_chr_read_poll, fd_chr_read, chr); @@ -1136,13 +1136,14 @@ static void pty_chr_state(CharDriverState *chr, int connected) if (!s-connected) { s-connected = 1; qemu_chr_be_generic_open(chr); +} +if (!chr-fd_in_tag) { chr-fd_in_tag = io_add_watch_poll(s-fd, pty_chr_read_poll, pty_chr_read, chr); } } } - static void pty_chr_close(struct CharDriverState *chr) { PtyCharDriver *s = chr-opaque; @@ -2228,7 +2229,7 @@ static void udp_chr_update_read_handler(CharDriverState *chr) { NetCharDriver *s = chr-opaque; -remove_fd_in_watch(chr); +assert(!chr-fd_in_tag); if (s-chan) { chr-fd_in_tag = io_add_watch_poll(s-chan, udp_chr_read_poll, udp_chr_read, chr); @@ -2510,6 +2511,17 @@ static void tcp_chr_connect(void *opaque) qemu_chr_be_generic_open(chr); } +static void tcp_chr_update_read_handler(CharDriverState *chr) +{ +TCPCharDriver *s = chr-opaque; + +assert(!chr-fd_in_tag); +if (s-chan) { +chr-fd_in_tag = io_add_watch_poll(s-chan, tcp_chr_read_poll, + tcp_chr_read, chr); +} +} + #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c; static void tcp_chr_telnet_init(int fd) { @@ -2665,6 +2677,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, chr-get_msgfd = tcp_get_msgfd; chr-chr_add_client = tcp_chr_add_client; chr-chr_add_watch = tcp_chr_add_watch; +chr-chr_update_read_handler = tcp_chr_update_read_handler; /* be isn't opened until we get a connection */ chr-explicit_be_open = true; -- 1.8.1.4
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
Il 11/12/2013 12:04, Gal Hammer ha scritto: Michael, True, I haven't figure it out yet, but the current status is that recover from sleep doesn't work. As far as I can tell it could be either: 1. piix4_reset shouldn't be call on resume. 2. memory_region_set_enabled (called in pm_io_space_update) shouldn't use config[0x80]. 3. the config[0x80] shouldn't be zero in piix4_reset (current solution). 4. something else? I'm not well familiar with the PIIX4 emulation and your help will be appreciated. The datasheet says that config[0x80] is reset to 0. The PIIX spec says that during S3 the chipset provides Shadow registers for standard AT write only registers to save and restore system state information These are just for the 825x (DMA controller, PIC, PIT). We do not emulate them and our BIOS does not support them. I was told that a few memory controller registers survive S3, which in our case would be the i440FX's PAM registers, but I don't think this register should be one of them. What guest is breaking and how? Does the guest usually initialize this register, or does the firmware (SeaBIOS) do that? If the latter, this could be a SeaBIOS bug instead. Paolo
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 12/17/2013 7:52 AM, Peter Maydell wrote: On 17 December 2013 13:42, Alexander Graf ag...@suse.de wrote: Softfloat really isn't my area of expertise and I'd prefer to see these patches go in through a different tree :). Peter, do you want to take care of this patch and Add float32_to_uint64()? Or at least give me your reviewed-by tag and I'll send it through my tree since the second one is a build dependency for VSX patches. I've also seen comments on folding in bugfixes for patches that occurred earlier. I presume this is one of these cases. Mind to shed some light on this? Yeah, these are on my to-review list (we need the fixes for aarch64 too :-)) but I'm waiting for Tom to post an updated patchset which has the complete set of softfloat fixes rather than having one patchset with a first version and a different patchset with a patch which fixes bugs in the first one... thanks -- PMM So it would seem to make the most sense to go back and do what I had done originally -- separate the softfloat fixes into their own patch set and make the VSX and P7 patches dependent on those. Agree?
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo
Re: [Qemu-devel] [PATCH] qemu-img: set nocow flag to new file
On Wed, Dec 11, 2013 at 09:29:54AM +0100, Stefan Hajnoczi wrote: On Tue, Dec 10, 2013 at 10:23:41PM +, Alex Bennée wrote: stefa...@redhat.com writes: On Mon, Nov 18, 2013 at 12:54:59PM +0800, Chunyan Liu wrote: 2013/11/15 Stefan Hajnoczi stefa...@gmail.com On Thu, Nov 14, 2013 at 04:15:28PM +0800, Chunyan Liu wrote: Set NOCOW flag to newly created images to solve performance issues on btrfs. snip This should be optional and I'm not sure it should be the default. Rationale: If you're on btrfs you probably expect the copy-on-write and snapshot features of the file system. We shouldn't silently disable that unless the user asks for it. snip When the NOCOW attribute is set on a file, reflink copying (aka file-level snapshots) do not work: $ cp --reflink test.img test-snapshot.img This produces EINVAL. It is a regression if qemu-img create suddenly starts breaking this standard btrfs feature for existing users. Please make it a .bdrv_create() option which is off by default to avoid breaking existing users' workflows/scripts. The result should be something like: $ qemu-img create test.img 8G # file has NOCOW cleared $ qemu-img create -o nocow=on test.img 8G # file has NOCOW set I agree we shouldn't break existing work flows. I wonder if it would OK for qemu-img to issue a warning (when not --quiet) when it detects creation of an image on a partition where performance may not be as expected due to COW behaviour. A warning could help or at least prompt users to consider switching to nocow. IMHO such warnings are not nice - if a user does not wish to use the 'nocow' option because they want to keep the ability to use file label snapshots they should not be subjected to a warning message forever more. I suggest this is something to document in the man page instead. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 18 December 2013 14:21, Tom Musta tommu...@gmail.com wrote So it would seem to make the most sense to go back and do what I had done originally -- separate the softfloat fixes into their own patch set and make the VSX and P7 patches dependent on those. Agree? Yeah, that works. But really what I need is to not have patch 1 adds function X; patch 2 fixes a bug that was introduced in patch 1. I don't so much care whether the softfloat patches are a freestanding set or a part of a PPC series, as long as the patches themselves are in a reviewable state. thanks -- PMM
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
On Wed, Dec 18, 2013 at 2:00 AM, Orit Wasserman owass...@redhat.com wrote: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? If you use the high-level API that provides posix like functions, such as nfs_open() then libnfs does. nfs_open()/nfs_open_async() takes a mode parameter and libnfs checks the O_SYNC flag in modes. By default libnfs will translate any nfs_write*() or nfs_pwrite*() to NFS/WRITE3+UNSTABLE that allows the server to just write to cache/memory. IF you specify O_SYNC in the mode argument to nfds_open/nfs_open_async then libnfs will flag this handle as sync and any calls to nfs_write/nfs_pwrite will translate to NFS/WRITE3+FILE_SYNC Calls to nfs_fsync is translated to NFS/COMMIT3 Of course, as for normal file I/O this is useful but not great since you can only control the sync vs async per open filehandle. Libnfs does also allow you to send raw rpc commands to the server and using this API you can control the sync behaviour for individual writes. This means you coould do something like * emulate SCSI to the guest. * if guest sends SCSI/WRITE* without any FUA bits set, then for that I/O you send a NFS3/WRITE+UNSTABLE * if guest sends SCSI/WRITE* with FUA bits set, then for that I/O you send NFS3/WRITE+FILE_SYNC and then the guest kernel can control for each individual write whether it is sync or not. But that is probably something that can wait until later and don't need to be part of the initial patch? If peter wants to do this in the future I can create a small writeup on how to mixin the two different APIs using the same context.
Re: [Qemu-devel] [PATCH] qdev-monitor: device_add crashes on non-device driver name, fix
Ping? arm...@redhat.com writes: From: Markus Armbruster arm...@redhat.com Watch this: $ upstream-qemu -nodefaults -S -display none -monitor stdio QEMU 1.7.50 monitor - type 'help' for more information (qemu) device_add rng-egd /work/armbru/qemu/qdev-monitor.c:491:qdev_device_add: Object 0x2089b00 is not an instance of type device Aborted (core dumped) Crashes because rng-egd exists, but isn't a subtype of TYPE_DEVICE. Broken in commit 18b6dad. Cc: qemu-sta...@nongnu.org Signed-off-by: Markus Armbruster arm...@redhat.com --- qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index dc37a43..90a0cea 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) } } -if (!oc) { +if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) { qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device type); return NULL; }
Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
Ping? arm...@redhat.com writes: From: Markus Armbruster arm...@redhat.com Obvious cleanups possible since we no longer have the special case of a non-qdevified controller. v2: * Dropped PATCH 1/10 ide: Break all non-qdevified controllers Andreas qdevified them since; thanks! * Series renamed from Drop code for non-qdevified IDE, and clean up * Trivially rebased Markus Armbruster (9): ide: Move IDEDevice pointer from IDEBus to IDEState ide: Use IDEState member dev for device connected test ide: Don't block-align IDEState member smart_selftest_data ide: Drop redundant IDEState member bs ide: Drop redundant IDEState geometry members ide: Drop redundant IDEState member version ide: Drop redundant IDEState member drive_serial_str ide: Drop redundant IDEState member model ide: Drop redundant IDEState member wwn hw/ide/ahci.c | 19 +++-- hw/ide/atapi.c | 39 + hw/ide/core.c | 235 +++- hw/ide/internal.h | 15 +--- hw/ide/macio.c | 26 +++--- hw/ide/microdrive.c | 2 +- hw/ide/piix.c | 4 - hw/ide/qdev.c | 50 ++- 8 files changed, 181 insertions(+), 209 deletions(-)
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On 18/12/2013 16:19, Paolo Bonzini wrote: The PIIX spec says that during S3 the chipset provides Shadow registers for standard AT write only registers to save and restore system state information These are just for the 825x (DMA controller, PIC, PIT). We do not emulate them and our BIOS does not support them. I was told that a few memory controller registers survive S3, which in our case would be the i440FX's PAM registers, but I don't think this register should be one of them. What guest is breaking and how? Does the guest usually initialize this register, or does the firmware (SeaBIOS) do that? If the latter, this could be a SeaBIOS bug instead. Both Windows and Linux guests are breaking when system is suspend. On system wakeup nothing occurs and the OS is not restored. I don't know the answer for the remaining questions. Thanks, Gal.
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. -- MST
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 12/18/2013 8:35 AM, Peter Maydell wrote: On 18 December 2013 14:21, Tom Musta tommu...@gmail.com wrote So it would seem to make the most sense to go back and do what I had done originally -- separate the softfloat fixes into their own patch set and make the VSX and P7 patches dependent on those. Agree? Yeah, that works. But really what I need is to not have patch 1 adds function X; patch 2 fixes a bug that was introduced in patch 1. I don't so much care whether the softfloat patches are a freestanding set or a part of a PPC series, as long as the patches themselves are in a reviewable state. thanks -- PMM OK makes sense. I will fold the bug fix back into this series and re-publish.
Re: [Qemu-devel] [PATCH v2 0/9] Clean up IDE after completion of qdevification
I had a brief look at this series. Dropping redundant fields certainly sounds good and after the lengthy QOM'ifications of IDE devices we seem to no longer break any devices, but whether to check for device or BlockDriverState sounds more like a block topic to me... Regards, Andreas Am 18.12.2013 15:54, schrieb Markus Armbruster: Ping? arm...@redhat.com writes: From: Markus Armbruster arm...@redhat.com Obvious cleanups possible since we no longer have the special case of a non-qdevified controller. v2: * Dropped PATCH 1/10 ide: Break all non-qdevified controllers Andreas qdevified them since; thanks! * Series renamed from Drop code for non-qdevified IDE, and clean up * Trivially rebased Markus Armbruster (9): ide: Move IDEDevice pointer from IDEBus to IDEState ide: Use IDEState member dev for device connected test ide: Don't block-align IDEState member smart_selftest_data ide: Drop redundant IDEState member bs ide: Drop redundant IDEState geometry members ide: Drop redundant IDEState member version ide: Drop redundant IDEState member drive_serial_str ide: Drop redundant IDEState member model ide: Drop redundant IDEState member wwn hw/ide/ahci.c | 19 +++-- hw/ide/atapi.c | 39 + hw/ide/core.c | 235 +++- hw/ide/internal.h | 15 +--- hw/ide/macio.c | 26 +++--- hw/ide/microdrive.c | 2 +- hw/ide/piix.c | 4 - hw/ide/qdev.c | 50 ++- 8 files changed, 181 insertions(+), 209 deletions(-) -- 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 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
On Wed, 18 Dec 2013 11:36:52 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 20:38, Anthony Liguori ha scritto: On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 00:26, Anthony Liguori ha scritto: Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. Can you explain what you mean? The question is whether hotpluggable as a property applies to all devices or not. I think Andreas asked me to provide hotpluggable property to distinguish hotpluggable vs not hotpluggable DimmDevice via qom interface. But hotplug is strictly a bus level concept. It's a sequence of events that correspond to what happens when you add a new device to a bus after power on. Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Do we need per instance ability to set hotpluggable property? For example board might want to mark some CPUs as not hotpluggable. Igor's interface applies just as well to the case of plugging a device at startup; I think separating the two makes little sense. And once you have cold-plug and hot-plug in qdev core, it makes sense to add unplug as well. Also because we already have surprise removal in qdev core (that's unparent) and we have some kind of unplug request support (device_del/dc-unplug). One possibility that remains is to put cold/hot-plug in a BusDevice class rather than in the core qdev: Device BusDevice-- can be cold/hot-plugged but this adds more complication. For example, the same CPU can be hotpluggable or not depending on the board model, should the superclass be Device or BusDevice. And if we ever have multi-CPU targets, with the core CPU not hotpluggable and additional hotpluggable ones (e.g. for GPUs) what would be the superclass of the common CPU superclass? The question is whether there can be code sharing without touching the base class. You could certainly have a HotpluggableBusState and then a HotpluggableDeviceState. Interfaces would be another option too. Interfaces are fine, but the question is who finds them and calls them. In this case, the discovery mechanism is a link property, and the calling mechanism is an explicit hook in the realized property. If we don't need per instance hotpluggable state and we can call interfaces from generic qdev/device code, then we would need at first only TYPE_HOTPLUGGABLE_BUS_DEVICE_IF and later for link based hotplug we could add just TYPE_HOTPLUGGABLE_DEVICE_IF. Difference would be in the way they get access to hotplug device link, former one will use bus for it and second some other way. If we had aspect-oriented programming, we would be marking join points instead of writing if (dev-foo) bar(dev-foo) conditionals. But the idea is the same. The general concern is about polluting widely used base classes. It's better if we can avoid adding things to DeviceState and Object whenever possible. I agree. At the same time we should make base classes as small as possible, but not smaller than that. Paolo
Re: [Qemu-devel] [PATCH] qdev-monitor: device_add crashes on non-device driver name, fix
Am 18.12.2013 15:54, schrieb Markus Armbruster: Ping? Already queued on qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next Sorry, did the patch processing offline on a train. ;) Thanks, Andreas arm...@redhat.com writes: From: Markus Armbruster arm...@redhat.com Watch this: $ upstream-qemu -nodefaults -S -display none -monitor stdio QEMU 1.7.50 monitor - type 'help' for more information (qemu) device_add rng-egd /work/armbru/qemu/qdev-monitor.c:491:qdev_device_add: Object 0x2089b00 is not an instance of type device Aborted (core dumped) Crashes because rng-egd exists, but isn't a subtype of TYPE_DEVICE. Broken in commit 18b6dad. Cc: qemu-sta...@nongnu.org Signed-off-by: Markus Armbruster arm...@redhat.com --- qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index dc37a43..90a0cea 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) } } -if (!oc) { +if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) { qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device type); return NULL; } -- 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] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 18 December 2013 15:31, Tom Musta tommu...@gmail.com wrote: OK makes sense. I will fold the bug fix back into this series and re-publish. If you can keep all the softfloat patches in one PPC series and all at the beginning of the series that would also assist. thanks -- PMM
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On 18/12/2013 16:22, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). I think we both made a mistake and the right name is PMREGMISC—MISCELLANEOUS POWER MANAGEMENT (FUNCTION 3) :-). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Gal.
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
Il 18/12/2013 16:48, Igor Mammedov ha scritto: Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Do we need per instance ability to set hotpluggable property? For example board might want to mark some CPUs as not hotpluggable. I think such fine-grained control could be handled from dc-unplug. Let's not do anything more than we need, until we need it. Right now, all we need to model is the fact that a device X can act as hotplug controller for multiple buses, X is not the parent of those buses, and we need to tell those buses about X. This is hotplug-handler in BusState. We also like to distinguish devices that only support -device (or are even only board-instantiatable) from devices that support device_add. This is dc-hotpluggable. It is not absolutely necessary, but it makes sense if plugging gets a more central place in qdev. This is the case after you add hotplug-handler. Interfaces would be another option too. Interfaces are fine, but the question is who finds them and calls them. In this case, the discovery mechanism is a link property, and the calling mechanism is an explicit hook in the realized property. If we don't need per instance hotpluggable state and we can call interfaces from generic qdev/device code, then we would need at first only TYPE_HOTPLUGGABLE_BUS_DEVICE_IF and later for link based hotplug we could add just TYPE_HOTPLUGGABLE_DEVICE_IF. Difference would be in the way they get access to hotplug device link, former one will use bus for it and second some other way. I think this is overengineered. What you have is flexible and decently clean. I don't think there's any need to go from the device to the bus and from there optionally to the handler. Most of the time you couldn't do anything really in the bus, you would have to call some sort of bus-specific callback (like SCSIBusInfo). It is then equivalent to set the bus's parent device as a (hot)plug handler and go straight from the device to the handler, as in your patches. Paolo
[Qemu-devel] [PATCH] qdev: Drop misleading qbus_free() function
Same reasoning as commit 02a5c4c97422b40034f31265e0f139f7846172a8 (qdev: Drop misleading qdev_free() function). The qbus_free() function removes the child from the namespace and decrements the reference count. It does not, however, guarantee to free the child since the refcount may still be held. Just call object_unparent() directly. Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- hw/core/qdev.c | 7 +-- hw/pci/pci_bridge.c| 2 +- include/hw/qdev-core.h | 2 -- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index e374a93..bbd780a 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -481,11 +481,6 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam return bus; } -void qbus_free(BusState *bus) -{ -object_unparent(OBJECT(bus)); -} - static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev) { BusClass *bc = BUS_GET_CLASS(bus); @@ -794,7 +789,7 @@ static void device_unparent(Object *obj) while (dev-num_child_bus) { bus = QLIST_FIRST(dev-child_bus); -qbus_free(bus); +object_unparent(OBJECT(bus)); } if (dev-realized) { object_property_set_bool(obj, false, realized, NULL); diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index f72872e..355c3e1 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -391,7 +391,7 @@ void pci_bridge_exitfn(PCIDevice *pci_dev) pci_bridge_region_cleanup(s, s-windows); memory_region_destroy(s-address_space_mem); memory_region_destroy(s-address_space_io); -/* qbus_free() is called automatically during device deletion */ +/* object_unparent() is called automatically during device deletion */ } /* diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index f2043a6..2f6e1d1 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -272,8 +272,6 @@ void qdev_reset_all(DeviceState *dev); void qbus_reset_all(BusState *bus); void qbus_reset_all_fn(void *opaque); -void qbus_free(BusState *bus); - /* This should go away once we get rid of the NULL bus hack */ BusState *sysbus_get_default(void); -- 1.8.4.2
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
Il 18/12/2013 16:59, Gal Hammer ha scritto: Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). I think we both made a mistake and the right name is PMREGMISC—MISCELLANEOUS POWER MANAGEMENT (FUNCTION 3) :-). Yeah. :) Still, both PMBA and PMREGMISC need to be initialized by SeaBIOS. Paolo
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
On Wed, Dec 18, 2013 at 04:48:09PM +0100, Igor Mammedov wrote: On Wed, 18 Dec 2013 11:36:52 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 20:38, Anthony Liguori ha scritto: On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 00:26, Anthony Liguori ha scritto: Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. Can you explain what you mean? The question is whether hotpluggable as a property applies to all devices or not. I think Andreas asked me to provide hotpluggable property to distinguish hotpluggable vs not hotpluggable DimmDevice via qom interface. But hotplug is strictly a bus level concept. It's a sequence of events that correspond to what happens when you add a new device to a bus after power on. Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Do we need per instance ability to set hotpluggable property? For example board might want to mark some CPUs as not hotpluggable. It could be useful. In real life same device can be on-board or on a plugin card. But it's not a must, we survived without this so far. So maybe start not supporting it, add later? Igor's interface applies just as well to the case of plugging a device at startup; I think separating the two makes little sense. And once you have cold-plug and hot-plug in qdev core, it makes sense to add unplug as well. Also because we already have surprise removal in qdev core (that's unparent) and we have some kind of unplug request support (device_del/dc-unplug). One possibility that remains is to put cold/hot-plug in a BusDevice class rather than in the core qdev: Device BusDevice-- can be cold/hot-plugged but this adds more complication. For example, the same CPU can be hotpluggable or not depending on the board model, should the superclass be Device or BusDevice. And if we ever have multi-CPU targets, with the core CPU not hotpluggable and additional hotpluggable ones (e.g. for GPUs) what would be the superclass of the common CPU superclass? The question is whether there can be code sharing without touching the base class. You could certainly have a HotpluggableBusState and then a HotpluggableDeviceState. Interfaces would be another option too. Interfaces are fine, but the question is who finds them and calls them. In this case, the discovery mechanism is a link property, and the calling mechanism is an explicit hook in the realized property. If we don't need per instance hotpluggable state and we can call interfaces from generic qdev/device code, then we would need at first only TYPE_HOTPLUGGABLE_BUS_DEVICE_IF and later for link based hotplug we could add just TYPE_HOTPLUGGABLE_DEVICE_IF. Difference would be in the way they get access to hotplug device link, former one will use bus for it and second some other way. If we had aspect-oriented programming, we would be marking join points instead of writing if (dev-foo) bar(dev-foo) conditionals. But the idea is the same. The general concern is about polluting widely used base classes. It's better if we can avoid adding things to DeviceState and Object whenever possible. I agree. At the same time we should make base classes as small as possible, but not smaller than that. Paolo
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote: On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less.
Re: [Qemu-devel] [PATCH] qdev: Drop misleading qbus_free() function
Am 18.12.2013 17:15, schrieb Stefan Hajnoczi: Same reasoning as commit 02a5c4c97422b40034f31265e0f139f7846172a8 (qdev: Drop misleading qdev_free() function). The qbus_free() function removes the child from the namespace and decrements the reference count. It does not, however, guarantee to free the child since the refcount may still be held. Just call object_unparent() directly. Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Thanks, nice and trivial, applied to qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next 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 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
On Wed, 18 Dec 2013 18:26:07 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Dec 18, 2013 at 04:48:09PM +0100, Igor Mammedov wrote: On Wed, 18 Dec 2013 11:36:52 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 20:38, Anthony Liguori ha scritto: On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/12/2013 00:26, Anthony Liguori ha scritto: Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. Can you explain what you mean? The question is whether hotpluggable as a property applies to all devices or not. I think Andreas asked me to provide hotpluggable property to distinguish hotpluggable vs not hotpluggable DimmDevice via qom interface. But hotplug is strictly a bus level concept. It's a sequence of events that correspond to what happens when you add a new device to a bus after power on. Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Do we need per instance ability to set hotpluggable property? For example board might want to mark some CPUs as not hotpluggable. It could be useful. In real life same device can be on-board or on a plugin card. But it's not a must, we survived without this so far. So maybe start not supporting it, add later? Yes, that's surely could be done later Paolo
[Qemu-devel] [PATCH] qdev-monitor-test: simplify using g_assert_cmpstr()
Use g_assert_cmpstr() instead of combining g_assert() and strcmp(3). This simplifies the code since we no longer have to play games to distinguish NULL from using (null). gcc extension haters will also be happy that ?: was dropped. Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- tests/qdev-monitor-test.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c index ba7f9cc..eefaab8 100644 --- a/tests/qdev-monitor-test.c +++ b/tests/qdev-monitor-test.c @@ -32,8 +32,9 @@ static void test_device_add(void) }}); g_assert(response); error = qdict_get_qdict(response, error); -g_assert(!strcmp(qdict_get_try_str(error, desc) ?: , - Device needs media, but drive is empty)); +g_assert_cmpstr(qdict_get_try_str(error, desc), +==, +Device needs media, but drive is empty); QDECREF(response); /* Delete the drive */ @@ -42,7 +43,7 @@ static void test_device_add(void) \command-line\: \drive_del drive0\ }}); g_assert(response); -g_assert(!strcmp(qdict_get_try_str(response, return) ?: (null), )); +g_assert_cmpstr(qdict_get_try_str(response, return), ==, ); QDECREF(response); /* Try to re-add the drive. This fails with duplicate IDs if a leaked @@ -53,8 +54,7 @@ static void test_device_add(void) \command-line\: \drive_add pci-addr=auto if=none,id=drive0\ }}); g_assert(response); -g_assert(!strcmp(qdict_get_try_str(response, return) ?: , - OK\r\n)); +g_assert_cmpstr(qdict_get_try_str(response, return), ==, OK\r\n); QDECREF(response); qtest_end(); -- 1.8.4.2
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
- Messaggio originale - Da: Michael S. Tsirkin m...@redhat.com A: marcel a marce...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com, Gal Hammer gham...@redhat.com, seab...@seabios.org, qemu-devel@nongnu.org Inviato: Mercoledì, 18 dicembre 2013 17:33:06 Oggetto: Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset. On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote: On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less. Or put an array of (bdf, offset, size, value) tuples somewhere in low memory, fill it at startup, and reproduce it blindly at S3 resume time. This is similar to what UEFI does. Paolo
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
On Wed, 18 Dec 2013 16:59:02 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 18/12/2013 16:48, Igor Mammedov ha scritto: Hotplugging a device is a special case of plugging a device. If a bus or device only supports cold-plug, that can be done using bc-allow_hotplug = false or dc-hotpluggable = false. Do we need per instance ability to set hotpluggable property? For example board might want to mark some CPUs as not hotpluggable. I think such fine-grained control could be handled from dc-unplug. Let's not do anything more than we need, until we need it. I had in mind a lightweight conversion of initial memory to DimmDevices for i386 target and disabling its unplug until initial memory unplug would be properly handled by board and SeaBIOS. Paolo
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, 2013-12-18 at 18:33 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote: On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less. The problem still remains, we have to use pci_bios_init_device that in turn uses the PCIDevices list. Thanks, Marcel
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, 2013-12-18 at 11:34 -0500, Paolo Bonzini wrote: - Messaggio originale - Da: Michael S. Tsirkin m...@redhat.com A: marcel a marce...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com, Gal Hammer gham...@redhat.com, seab...@seabios.org, qemu-devel@nongnu.org Inviato: Mercoledì, 18 dicembre 2013 17:33:06 Oggetto: Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset. On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote: On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less. Or put an array of (bdf, offset, size, value) tuples somewhere in low memory, fill it at startup, and reproduce it blindly at S3 resume time. This is similar to what UEFI does. Could you please elaborate a little more? Let me see first if I understand the problem: PciDevices list is a list of pointers that cannot be used inside init code which is 16 bit code, right? If I got it right, the solution is to find another way to iterate over pci devices? If yes, *when* would this data be put in memory and when? A pointer to the right direction would be appreciated, Thanks! Marcel Paolo
Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset.
On Wed, Dec 18, 2013 at 11:34:14AM -0500, Paolo Bonzini wrote: - Messaggio originale - Da: Michael S. Tsirkin m...@redhat.com A: marcel a marce...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com, Gal Hammer gham...@redhat.com, seab...@seabios.org, qemu-devel@nongnu.org Inviato: Mercoledì, 18 dicembre 2013 17:33:06 Oggetto: Re: [Qemu-devel] [PATCH] piix: do not reset APIC base address (0x80) on piix4_reset. On Wed, Dec 18, 2013 at 06:27:12PM +0200, Marcel Apfelbaum wrote: On Wed, 2013-12-18 at 17:22 +0200, Michael S. Tsirkin wrote: On Wed, Dec 18, 2013 at 03:22:59PM +0100, Paolo Bonzini wrote: Il 11/12/2013 10:21, Gal Hammer ha scritto: Fix a bug that was introduced in commit c046e8c4. QEMU fails to resume from suspend mode (S3). Signed-off-by: Gal Hammer gham...@redhat.com --- hw/acpi/piix4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 93849c8..5c736a4 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -376,7 +376,6 @@ static void piix4_reset(void *opaque) pci_conf[0x5b] = 0; pci_conf[0x40] = 0x01; /* PM io base read only bit */ -pci_conf[0x80] = 0; if (s-kvm_enabled) { /* Mark SMM as already inited (until KVM supports SMM). */ Note this is not the APIC base address, that one is 80h on the ISA bridge (function 0). You're changing the behavior for 80h on the power management function, which is function 3. The register is PMBA—POWER MANAGEMENT BASE ADDRESS and it is indeed initialized by SeaBIOS in piix4_pm_setup (src/fw/pciinit.c). Michael, perhaps a part of pci_setup (same file) should run on S3 resume? Paolo Seems reasonable: either seabios or guest OS must do it, and guest does not seem to. I was looking into this today, but it seems that we have a problem. We cannot run pci_setup() in init section: .data.varinit.seabios/src/hw/pci.h.66 is VARVERIFY32INIT but used from ['.text.runtime.seabios/src/resume.c.150', '.text.pci_setup'] Any thoughts how to get around this? Thanks, Marcel We defintely don't want to do full pci enumeration. Just pci_bios_init_platform or even less. Or put an array of (bdf, offset, size, value) tuples somewhere in low memory, fill it at startup, and reproduce it blindly at S3 resume time. This is similar to what UEFI does. Paolo Yes, it's an option. Though reworking pci_bios_init_devices so that it can work from low memory seems easier.
Re: [Qemu-devel] [PATCH v2 0/5] Monitor commands for object-add/del
On Tue, 17 Dec 2013 12:25:59 +0100 Paolo Bonzini pbonz...@redhat.com wrote: These allow hotplugging (and hot-unplugging without leaking an object) virtio-rng devices. They can also be used for memory hotplug. v1-v2: fix mistyped underscores in qapi-schema.json Applied to the qmp branch, thanks. Paolo Bonzini (5): rng: initialize file descriptor to -1 qom: fix leak for objects created with -object qom: catch errors in object_property_add_child monitor: add object-add (QMP) and object_add (HMP) command monitor: add object-del (QMP) and object_del (HMP) command backends/rng-random.c | 4 +-- hmp-commands.hx | 28 ++ hmp.c | 67 ++ hmp.h | 2 ++ include/monitor/monitor.h | 3 ++ include/qapi/visitor.h| 3 +- include/qemu/typedefs.h | 2 ++ qapi-schema.json | 34 ++ qmp-commands.hx | 51 qmp.c | 74 +++ qom/object.c | 9 -- vl.c | 3 +- 12 files changed, 273 insertions(+), 7 deletions(-)
Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
Am 18.12.2013 um 15:42 schrieb ronnie sahlberg ronniesahlb...@gmail.com: On Wed, Dec 18, 2013 at 2:00 AM, Orit Wasserman owass...@redhat.com wrote: On 12/18/2013 01:03 AM, Peter Lieven wrote: Am 17.12.2013 um 18:32 schrieb Daniel P. Berrange berra...@redhat.com: On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote: This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs://host/export/filename For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 Does it support other config tunables, eg specifying which NFS version to use 2/3/4 ? If so will they be available as URI parameters in the obvious manner ? currently only v3 is supported by libnfs. what other tunables would you like to see? For live migration we need the sync option (async ignores O_SYNC and O_DIRECT sadly), will it be supported? or will it be the default? If you use the high-level API that provides posix like functions, such as nfs_open() then libnfs does. nfs_open()/nfs_open_async() takes a mode parameter and libnfs checks the O_SYNC flag in modes. By default libnfs will translate any nfs_write*() or nfs_pwrite*() to NFS/WRITE3+UNSTABLE that allows the server to just write to cache/memory. IF you specify O_SYNC in the mode argument to nfds_open/nfs_open_async then libnfs will flag this handle as sync and any calls to nfs_write/nfs_pwrite will translate to NFS/WRITE3+FILE_SYNC Calls to nfs_fsync is translated to NFS/COMMIT3 If this NFS/COMMIT3 would issue a sync on the server that would be all we actually need. And in this case migration should be safe. Even if we open a file with cache = none qemu would issue such a commit after every write. This also allow for writeback caching where the filesystem flushes would go through right to the server. Of course, as for normal file I/O this is useful but not great since you can only control the sync vs async per open filehandle. Libnfs does also allow you to send raw rpc commands to the server and using this API you can control the sync behaviour for individual writes. This means you coould do something like * emulate SCSI to the guest. * if guest sends SCSI/WRITE* without any FUA bits set, then for that I/O you send a NFS3/WRITE+UNSTABLE * if guest sends SCSI/WRITE* with FUA bits set, then for that I/O you send NFS3/WRITE+FILE_SYNC and then the guest kernel can control for each individual write whether it is sync or not. But that is probably something that can wait until later and don't need to be part of the initial patch? If peter wants to do this in the future I can create a small writeup on how to mixin the two different APIs using the same context. We can do that, but I would like to focus on the basic functionality first. Peter
[Qemu-devel] [PATCH] seccomp: exit if seccomp_init() fails
This fixes a bug where we weren't exiting if seccomp_init() failed. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- qemu-seccomp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qemu-seccomp.c b/qemu-seccomp.c index cf07869..b7c1253 100644 --- a/qemu-seccomp.c +++ b/qemu-seccomp.c @@ -231,6 +231,7 @@ int seccomp_start(void) ctx = seccomp_init(SCMP_ACT_KILL); if (ctx == NULL) { +rc = -1; goto seccomp_return; } -- 1.8.1.4
[Qemu-devel] [PULL 11/13] monitor: add object-add (QMP) and object_add (HMP) command
From: Paolo Bonzini pbonz...@redhat.com Add two commands that are the monitor counterparts of -object. The commands have the same Visitor-based implementation, but use different kinds of visitors so that the HMP command has a DWIM string-based syntax, while the QMP variant accepts a stricter JSON-based properties dictionary. Signed-off-by: Paolo Bonzini pbonz...@redhat.com Reviewed-By: Igor Mammedov imamm...@redhat.com Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- hmp-commands.hx | 14 +++ hmp.c | 58 hmp.h | 1 + include/monitor/monitor.h | 3 +++ include/qapi/visitor.h| 3 +-- include/qemu/typedefs.h | 2 ++ qapi-schema.json | 20 +++ qmp-commands.hx | 27 + qmp.c | 62 +++ 9 files changed, 188 insertions(+), 2 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index ebe8e78..2951d1e 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1243,6 +1243,20 @@ STEXI Remove host network device. ETEXI +{ +.name = object_add, +.args_type = object:O, +.params = [qom-type=]type,id=str[,prop=value][,...], +.help = create QOM object, +.mhandler.cmd = hmp_object_add, +}, + +STEXI +@item object_add +@findex object_add +Create QOM object. +ETEXI + #ifdef CONFIG_SLIRP { .name = hostfwd_add, diff --git a/hmp.c b/hmp.c index 32ee285..a1669ab 100644 --- a/hmp.c +++ b/hmp.c @@ -21,6 +21,7 @@ #include qmp-commands.h #include qemu/sockets.h #include monitor/monitor.h +#include qapi/opts-visitor.h #include ui/console.h #include block/qapi.h #include qemu-io.h @@ -1354,6 +1355,63 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, err); } +void hmp_object_add(Monitor *mon, const QDict *qdict) +{ +Error *err = NULL; +QemuOpts *opts; +char *type = NULL; +char *id = NULL; +void *dummy = NULL; +OptsVisitor *ov; +QDict *pdict; + +opts = qemu_opts_from_qdict(qemu_find_opts(object), qdict, err); +if (error_is_set(err)) { +goto out; +} + +ov = opts_visitor_new(opts); +pdict = qdict_clone_shallow(qdict); + +visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err); +if (error_is_set(err)) { +goto out_clean; +} + +qdict_del(pdict, qom-type); +visit_type_str(opts_get_visitor(ov), type, qom-type, err); +if (error_is_set(err)) { +goto out_clean; +} + +qdict_del(pdict, id); +visit_type_str(opts_get_visitor(ov), id, id, err); +if (error_is_set(err)) { +goto out_clean; +} + +object_add(type, id, pdict, opts_get_visitor(ov), err); +if (error_is_set(err)) { +goto out_clean; +} +visit_end_struct(opts_get_visitor(ov), err); +if (error_is_set(err)) { +qmp_object_del(id, NULL); +} + +out_clean: +opts_visitor_cleanup(ov); + +QDECREF(pdict); +qemu_opts_del(opts); +g_free(id); +g_free(type); +g_free(dummy); + +out: +hmp_handle_error(mon, err); +} + void hmp_getfd(Monitor *mon, const QDict *qdict) { const char *fdname = qdict_get_str(qdict, fdname); diff --git a/hmp.h b/hmp.h index 54cf71f..521449b 100644 --- a/hmp.h +++ b/hmp.h @@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict); void hmp_chardev_add(Monitor *mon, const QDict *qdict); void hmp_chardev_remove(Monitor *mon, const QDict *qdict); void hmp_qemu_io(Monitor *mon, const QDict *qdict); +void hmp_object_add(Monitor *mon, const QDict *qdict); #endif diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 10fa0e3..22d8b8f 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func, int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret); int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret); +int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret); +void object_add(const char *type, const char *id, const QDict *qdict, +Visitor *v, Error **errp); AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, bool has_opaque, const char *opaque, diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 48a2a2e..29da211 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -13,6 +13,7 @@ #ifndef QAPI_VISITOR_CORE_H #define QAPI_VISITOR_CORE_H +#include qemu/typedefs.h #include qapi/qmp/qobject.h #include qapi/error.h #include stdlib.h @@ -26,8 +27,6 @@ typedef struct GenericList struct GenericList *next; } GenericList; -typedef struct Visitor Visitor; - void visit_start_handle(Visitor *v, void **obj, const char *kind,