Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] target-ppc: Handle ibm, nmi-register RTAS call
On Monday 08 September 2014 02:17 AM, Alexander Graf wrote:
On 25.08.14 15:45, Aravinda Prasad wrote:
This patch adds FWNMI support in qemu for powerKVM
guests by handling the ibm,nmi-register rtas call.
Whenever OS issues ibm,nmi-register RTAS call, the
machine check notification address is saved and the
machine check interrupt vector 0x200 is patched to
issue a private hcall.
Signed-off-by: Aravinda Prasad aravi...@linux.vnet.ibm.com
---
hw/ppc/spapr_rtas.c| 91
include/hw/ppc/spapr.h |8
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 02ddbf9..1135d2b 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -277,6 +277,91 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU
*cpu,
rtas_st(rets, 0, ret);
}
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+int i;
+uint32_t branch_inst = 0x4802;
+target_ulong guest_machine_check_addr;
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+/*
+ * Trampoline saves r3 in sprg2 and issues private hcall
+ * to request qemu to build error log. QEMU builds the
+ * error log, copies to rtas-blob and returns the address.
+ * The initial 16 bytes in rtas-blob consists of saved srr0
+ * and srr1 which we restore and pass on the actual error
+ * log address to OS handled mcachine check notification
+ * routine
+ */
+uint32_t trampoline[] = {
+0x7c7243a6,/* mtspr SPRN_SPRG2,r3 */
+0x3860,/* li r3,0 */
+/* 0xf004 is the KVMPPC_H_REPORT_ERR private HCALL */
+0x6063f004,/* ori r3,r3,f004 */
+/* Issue H_CALL */
+0x4422,/* sc 1 */
+0x7c9243a6,/* mtspr r4 sprg2 */
+0xe883,/* ld r4, 0(r3) */
+0x7c9a03a6,/* mtspr r4, srr0 */
+0xe8830008,/* ld r4, 8(r3) */
+0x7c9b03a6,/* mtspr r4, srr1 */
+0x38630010,/* addi r3,r3,16 */
+0x7c9242a6,/* mfspr r4 sprg2 */
+0x4802,/* Branch to address registered
+* by OS. The branch address is
+* patched below */
+0x4800,/* b . */
So how about we just completely change the layout of the RTAS blob?
Imagine something like the following (completely untested):
/ index table /
.long rtas_entry
.long nmi_register
.long nmi_register_final_branch
.long nmi_data
/ RTAS handling code /
.align 1024
rtas_entry:
...
nmi_register:
...
nmi_register_final_branch:
ba .
/ RTAS data regions /
.align 4096
nmi_data:
.long 0
.align 4096
With this we should be able to have a nice hybrid between easily tunable
asm code and an easy to load and handle blob.
Sorry, I was out of office hence could not respond.
Yes, even I prefer something like this.
BTB, did you intend to have this in spapr-rtas.S? The spapr-rtas.S is
compiled into a binary and is read into spapr-rtas_blob. If we want to
have rtas-blob layout something similar to above then we may need to
link the object file of spapr-rtas.S to QEMU so that the symbols in
index table and other places could be resolved inside QEMU.
If this is fine I will include it in v3.
Regards,
Aravinda
Alex
--
Regards,
Aravinda
[Qemu-devel] [PATCH 0/2] guest-agent: make CPU's online/offline state persistent
Make sure that VCPU online state managed with virsh setvcpus --guest is not lost on reboot. Igor Mammedov (2): guest-agent: keep persistent state on persistent storage guest-agent: preserve online/offline state of VCPUs on guest reboot qga/main.c | 107 +++-- 1 file changed, 105 insertions(+), 2 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 2/2] guest-agent: preserve online/offline state of VCPUs on guest reboot
Fixes issue when CPU was offlined via libvirt using command:
virsh setvcpus --guest myguest NR_CPUS
but it became onlined again after guest was rebooted.
Fix issue by storing current state of CPUs online state
on persistent storage when GA is being stopped and restore
it when it's started at system boot time.
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
qga/main.c | 102 +
1 file changed, 102 insertions(+)
diff --git a/qga/main.c b/qga/main.c
index 5afba01..1173ca9 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -32,6 +32,7 @@
#include qapi/qmp/dispatch.h
#include qga/channel.h
#include qemu/bswap.h
+#include qga-qmp-commands.h
#ifdef _WIN32
#include qga/service-win32.h
#include qga/vss-win32.h
@@ -57,6 +58,7 @@
#define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR /fsfreeze-hook
#endif
#define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CPU_STATE_GROUP CPU online states
static struct {
const char *state_dir;
@@ -66,6 +68,7 @@ static struct {
typedef struct GAPersistentState {
#define QGA_PSTATE_DEFAULT_FD_COUNTER 1000
int64_t fd_counter;
+GuestLogicalProcessorList *vcpus;
} GAPersistentState;
struct GAState {
@@ -770,6 +773,40 @@ static void
persistent_state_from_keyfile(GAPersistentState *pstate,
pstate-fd_counter =
g_key_file_get_integer(keyfile, global, fd_counter, NULL);
}
+
+if (g_key_file_has_group(keyfile, QGA_CPU_STATE_GROUP)) {
+bool state;
+char **cpu_id_str;
+GuestLogicalProcessor *vcpu;
+GuestLogicalProcessorList *entry;
+GuestLogicalProcessorList *head, **link;
+char **cpus_list = g_key_file_get_keys(keyfile, QGA_CPU_STATE_GROUP,
+ NULL, NULL);
+
+head = NULL;
+link = head;
+for (cpu_id_str = cpus_list; *cpu_id_str; ++cpu_id_str) {
+state = g_key_file_get_boolean(keyfile, QGA_CPU_STATE_GROUP,
+ *cpu_id_str, NULL);
+
+vcpu = g_malloc0(sizeof *vcpu);
+vcpu-logical_id = g_ascii_strtoll(*cpu_id_str, NULL, 0);
+vcpu-online = state;
+vcpu-has_can_offline = true;
+
+entry = g_malloc0(sizeof *entry);
+entry-value = vcpu;
+
+*link = entry;
+link = entry-next;
+}
+pstate-vcpus = head;
+
+g_strfreev(cpus_list);
+}
+if (pstate-vcpus == NULL) {
+pstate-vcpus = qmp_guest_get_vcpus(NULL);
+}
}
static void persistent_state_to_keyfile(const GAPersistentState *pstate,
@@ -779,6 +816,27 @@ static void persistent_state_to_keyfile(const
GAPersistentState *pstate,
g_assert(keyfile);
g_key_file_set_integer(keyfile, global, fd_counter,
pstate-fd_counter);
+
+if (pstate-vcpus) {
+GuestLogicalProcessorList *vcpus = pstate-vcpus;
+
+while (vcpus != NULL) {
+char *logical_id;
+GuestLogicalProcessor *vcpu = vcpus-value;
+
+if (vcpu-can_offline == false) {
+vcpus = vcpus-next;
+continue;
+}
+
+logical_id = g_strdup_printf(% PRId64, vcpu-logical_id);
+g_key_file_set_boolean(keyfile, QGA_CPU_STATE_GROUP,
+ logical_id, vcpu-online);
+g_free(logical_id);
+
+vcpus = vcpus-next;
+}
+}
}
static gboolean write_persistent_state(const GAPersistentState *pstate,
@@ -820,6 +878,47 @@ out:
return ret;
}
+static void ga_clean_vcpu_pstate(GAPersistentState *pstate)
+{
+if (pstate-vcpus) {
+qapi_free_GuestLogicalProcessorList(pstate-vcpus);
+pstate-vcpus = NULL;
+}
+}
+
+static void ga_clean_pstate(GAPersistentState *pstate)
+{
+ga_clean_vcpu_pstate(pstate);
+}
+
+#if defined(__linux__)
+
+static void ga_update_vcpu_pstate(GAPersistentState *pstate, const char *path)
+{
+Error *local_err = NULL;
+
+ga_clean_vcpu_pstate(pstate);
+pstate-vcpus = qmp_guest_get_vcpus(local_err);
+
+if (local_err != NULL) {
+g_critical(%s, error_get_pretty(local_err));
+error_free(local_err);
+return;
+}
+
+if (!write_persistent_state(pstate, path)) {
+g_critical(failed to commit CPU persistent state to disk);
+}
+}
+
+#else
+
+static void ga_update_vcpu_pstate(GAPersistentState *pstate, const char *path)
+{
+}
+
+#endif
+
static gboolean read_persistent_state(GAPersistentState *pstate,
const gchar *path, gboolean frozen)
{
@@ -878,6 +977,7 @@ static gboolean read_persistent_state(GAPersistentState
*pstate,
}
persistent_state_from_keyfile(pstate, keyfile);
+qmp_guest_set_vcpus(pstate-vcpus, error_abort);
out:
if (keyfile) {
@@ -1185,6 +1285,8 @@ int main(int argc, char **argv)
ga_command_state_cleanup_all(ga_state-command_state);
[Qemu-devel] [PATCH 1/2] guest-agent: keep persistent state on persistent storage
GA was keepeing persistent state info in /var/run/qga.state file. However it's lost after every reboot since /var/run usually is located on tmpfs or cleaned on start-up. Fix issue by keeping state file in /var/lib/qemu-ga/ directory, which is intended for keeping persistent local state according to FHS. Signed-off-by: Igor Mammedov imamm...@redhat.com --- qga/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qga/main.c b/qga/main.c index 227f2bd..5afba01 100644 --- a/qga/main.c +++ b/qga/main.c @@ -45,7 +45,8 @@ #ifndef _WIN32 #define QGA_VIRTIO_PATH_DEFAULT /dev/virtio-ports/org.qemu.guest_agent.0 -#define QGA_STATE_RELATIVE_DIR run +#define QGA_VOLATILE_STATE_RELATIVE_DIR run +#define QGA_STATE_RELATIVE_DIR lib/qemu-ga #define QGA_SERIAL_PATH_DEFAULT /dev/ttyS0 #else #define QGA_VIRTIO_PATH_DEFAULT .\\Global\\org.qemu.guest_agent.0 @@ -121,7 +122,7 @@ init_dfl_pathnames(void) dfl_pathnames.state_dir = qemu_get_local_state_pathname( QGA_STATE_RELATIVE_DIR); dfl_pathnames.pidfile = qemu_get_local_state_pathname( - QGA_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S qemu-ga.pid); + QGA_VOLATILE_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S qemu-ga.pid); } static void quit_handler(int sig) -- 1.8.3.1
Re: [Qemu-devel] Fullscreen Bug with GTK
Hi, There is a bug where the GTK (SDL appears to be broken due to some sdl2 incompatibility, as I understood) menu bar won't hide in fullscreen: SDL-1 support is still there. https://bugs.launchpad.net/qemu/+bug/1294898 A patch was provided by the initial reporter a long time ago. Patch trades one bug for another. Hiding the menu bar also kills the accelerator keys, which is especially problematic for the fullscreen hotkey as there is no way out of fullscreen mode then. cheers, Gerd
Re: [Qemu-devel] [PATCH 10/17] mm: rmap preparation for remap_anon_pages
* Linus Torvalds (torva...@linux-foundation.org) wrote: On Fri, Oct 3, 2014 at 10:08 AM, Andrea Arcangeli aarca...@redhat.com wrote: Overall this looks a fairly small change to the rmap code, notably less intrusive than the nonlinear vmas created by remap_file_pages. Considering that remap_file_pages() was an unmitigated disaster, and -mm has a patch to remove it entirely, I'm not at all convinced this is a good argument. We thought remap_file_pages() was a good idea, and it really really really wasn't. Almost nobody used it, why would the anonymous page case be any different? I've posted code that uses this interface to qemu-devel and it works nicely; so chalk up at least one user. For the postcopy case I'm using it for, we need to place a page, atomically some thread might try and access it, and must either 1) get caught by userfault etc or 2) must succeed in it's access and we'll have that happening somewhere between thousands and millions of times to pages in no particular order, so we need to avoid creating millions of mappings. Dave Linus -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [Bug 638955] Re: emulated netcards don't work with recent sunos kernel
On Sun, Oct 5, 2014 at 9:57 PM, dblade listm...@triad.rr.com wrote: I have this problem (as describe in OP) on a Solaris 11.2 install using the text iso. Archlinux Qemu 2.1.0. It appears that the above patch has been applied to qemu for some time now (its also in my version). Are there any new workarounds? Hi, It's been a long time since that fix was developed. At this point it would be necessary to debug the problem from scratch. I don't have time to work on this in the near future, sorry. Maybe someone else wants to figure out what is wrong. Stefan -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/638955 Title: emulated netcards don't work with recent sunos kernel Status in QEMU: New Bug description: hi there, i'm using qemu-kvm backend in version: # qemu-kvm -version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 Fabrice Bellard and there are just *not working any of model=$type with combinations of recent sunos (solaris, openindiana, opensolaris, ..) .. you can download for testing purposes iso from here: http://dlc- origin.openindiana.org/isos/147/ or from here: http://genunix.org/distributions/indiana/ osol and oi are also bubuntu-like *live cds, so no need to bother with installing behaviour is as follows: e1000 - receiving doesn't work, transmitting works .. dladm (tool for handle ethers) shows that is all ok, correct mode is loaded up, it just seems like this driver works at 100% but .. rtl8169|pcnet - works in 10Mbit mode with several other issues like high cpu utilization and so .. dladm is unable to recognize options for this kind of -nic others - just don't work .. i experienced this issue several times in past .. woraround was, that rtl8169 worked so-so .. with recent sunos kernel it doesn't. it's easy to reproduce, this is why i'm not putting here more then launching script for my virtual machine: # cat openindiana.sh qemu-kvm -hda /home/kvm/openindiana/openindiana.img -m 2048 -localtime -cdrom /home/kvm/+images/oi-dev-147-x86.iso -boot d \ -vga std -vnc :9 -k en-us -monitor unix:/home/kvm/openindiana/instance,server,nowait \ -net nic,model=e1000,vlan=1 -net tap,ifname=oi0,script=no,vlan=1 sleep 2; ip l set oi0 up; ip a a 192.168.99.9/24 dev oi0; regards by daniel To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/638955/+subscriptions
Re: [Qemu-devel] [PATCH 1/3] libqos: Remove PCI assumptions in virtio driver
On Fri, Oct 03, 2014 at 01:53:13PM +0200, Marc Marí wrote:
El Thu, 2 Oct 2014 13:02:25 +0100
Stefan Hajnoczi stefa...@redhat.com escribió:
On Thu, Sep 04, 2014 at 06:24:37PM +0200, Marc Marí wrote:
@@ -60,25 +60,25 @@ static void
qvirtio_pci_assign_device(QVirtioDevice *d, void *data) *vpcidev =
(QVirtioPCIDevice *)d; }
-static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void
*addr) +static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d,
uint64_t addr) {
QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-return qpci_io_readb(dev-pdev, addr);
+return qpci_io_readb(dev-pdev, (void *)addr);
You do not need casts in C for void* to any pointer type or any
pointer type to void*. Please drop them.
addr is of type uint64_t, not a pointer. So if there's no cast it will
fail to compile (expected ‘void *’ but argument is of type ‘uint64_t’)
Hmm...on 32-bit hosts that causes an error:
a.c:7:17: warning: cast to pointer from integer of different size
[-Wint-to-pointer-cast]
printf(%p\n, (void*)u64);
You need to cast to uintptr_t first:
(void*)(uintptr_t)addr
This assumes that PCI IO Space addresses are 32-bit when running 32-bit
hosts. Not sure whether or not that's true, but at least for x86 it
works since the legacy PIO address space is 16-bit.
Stefan
pgpXV1y11jlrF.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
On Wed, Sep 17, 2014 at 02:39:50PM +0300, Gal Hammer wrote: Hi, A two parts patch to add a QEmu support for Microsoft's Virtual Machine Generation ID device. The first one add a new ACPI directive which allow to use a 16-bytes buffer in an ACPI table. This buffer is for storing the VM's UUID. The second is the ACPI tables changes and the actual device. Your comment are welcomed. There are some rules on when the VM generation ID *must* change Virtual machine is paused or resumed: No Virtual machine reboots: No Virtual machine host reboots: No Virtual machine starts executing a snapshot (every time): Yes Virtual machine is recovered from backup: Yes Virtual machine is failed over in a disaster recovery environment: Yes Virtual machine is live migrated: No Virtual machine is imported, copied, or cloned: Yes Virtual machine is failed over in a clustered environment: No Virtual machine's configuration changes: Unspecified Now this can largely be accomplished by libvirt by simply changing the value of the -vmgenid command line parameter, because most of these scenarios involve the spawning of a new QEMU process. The exception I think is when a running guest is reverted to a previous snapshot, because that is done via a monitor command and not restarting QEMU. So for this VM Generation ID work to be considered complete we need to have a way to dynamically change the VM generation ID on the fly, atomatically with reverting snapshots from the POV of the guest. eg we must load the snapshot state, change the generation ID, and only then start CPUs again. IOW I think this patch series is incomplete wrt the Microsoft spec on generation ID semantics. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Fullscreen Bug with GTK
* Gerd Hoffmann (kra...@redhat.com) wrote: Hi, There is a bug where the GTK (SDL appears to be broken due to some sdl2 incompatibility, as I understood) menu bar won't hide in fullscreen: SDL-1 support is still there. Are you recommending building against SDL-1 even when SDL-2 is available? (Does that lose you anything?) (I've cc'd Cole in since Fedora's virt-preview is building it with SDL-2). https://bugs.launchpad.net/qemu/+bug/1294898 A patch was provided by the initial reporter a long time ago. Patch trades one bug for another. Hiding the menu bar also kills the accelerator keys, which is especially problematic for the fullscreen hotkey as there is no way out of fullscreen mode then. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 1/8] virtio-gpu/2d: add hardware spec include file
Hi, +VIRTIO_GPU_FORMAT_B5G5R5A1_UNORM = 5, +VIRTIO_GPU_FORMAT_B5G6R5_UNORM= 7, Ok. But for 2D we can just not support it, right? We can, I expect some pushback at some point, people still want to test with 16bpp for other areas, and it would be nice to know they can. But I don't really care about it personally. I just though we should provide at least a basic number of working bpps (8,16,32). Lets try to get away with 32bpp only in 2d mode then. bochsdrm likewise supports 32bpp only and I yet have to see a request for 16bpp or even 8bpp support. I think we should probably move a few more formats from the 3D side into the 2D side, so we can have the guests just pick the LE format it requires http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/include/pipe/p_format.h#n354 is what gallium currently does, and we could just provide XRGB, XBGR formats in both endianness and have the guest pick the one it wants to use. PIPE_FORMAT_R8G8B8A8_UNORM = 67, PIPE_FORMAT_X8B8G8R8_UNORM = 68, PIPE_FORMAT_A8B8G8R8_UNORM = 121, PIPE_FORMAT_R8G8B8X8_UNORM = 134, With the last two ones being in a /* TODO: re-order these */ block. How stable are these numbers? The 2D pixman code would need updating to provide 2D support for these formats as well. Yep. Mapping the 32bpp formats to pixmap formats is easy. I suspect I could add an endian cap for the 3D bits that I could pass through from guest to host. I was thinking about using virtio feature bit. Advantage is that the guest will tell the host which features it'll use. Initially this doesn't matter much as the host will support only one endianness anyway. But in case we get the byteswapping work reasonable well some day and the host supports both be and le virgl we'll know that way which endianness the guest is using. How do you test guests with big endian? Isn't it really slow? emulated pseries machine with fedora ppc64. Yes, it is slow. Building a kernel with virtio-gpu driver takes a day or so. cheers, Gerd
Re: [Qemu-devel] [PATCH v2 1/1] xen-hvm.c: Add support for Xen access to vmport
On Fri, 3 Oct 2014, Don Slutz wrote:
On 10/03/14 12:23, Stefano Stabellini wrote:
On Fri, 3 Oct 2014, Don Slutz wrote:
On 10/03/14 05:52, Stefano Stabellini wrote:
On Thu, 2 Oct 2014, Don Slutz wrote:
This adds synchronisation of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.
...
}
-static void handle_ioreq(ioreq_t *req)
+static void regs_to_cpu(XenIOState *state, vmware_ioreq_t
*vmport_req)
+{
+X86CPU *cpu;
+CPUX86State *env;
+
+if (!state-cpu_by_ioreq_id[0]) {
+CPUState *cpu_state;
+
+CPU_FOREACH(cpu_state) {
+state-cpu_by_ioreq_id[cpu_state-cpu_index] = cpu_state;
+}
+}
This is just the initialization, isn't it?
It would be best to move it to an initialization function then.
A new initialization function would need to be added. A new call to it
would
need
to be added (not sure where the best place is). Since the overhead here
is
small I went with the less intrusive change.
I'd prefer the initialization function if it is possible.
Ok, how does:
@@ -1023,6 +1028,11 @@ static void xen_main_loop_prepare(XenIOState *state)
state);
if (evtchn_fd != -1) {
+CPUState *cpu_state;
+
+CPU_FOREACH(cpu_state) {
+state-cpu_by_ioreq_id[cpu_state-cpu_index] = cpu_state;
+}
qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state);
}
}
Look?
It looks fine
Re: [Qemu-devel] [PATCH 1/2] guest-agent: keep persistent state on persistent storage
On 10/06/14 09:44, Igor Mammedov wrote: GA was keepeing persistent state info in /var/run/qga.state file. However it's lost after every reboot since /var/run usually is located on tmpfs or cleaned on start-up. Fix issue by keeping state file in /var/lib/qemu-ga/ directory, which is intended for keeping persistent local state according to FHS. Signed-off-by: Igor Mammedov imamm...@redhat.com --- qga/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qga/main.c b/qga/main.c index 227f2bd..5afba01 100644 --- a/qga/main.c +++ b/qga/main.c @@ -45,7 +45,8 @@ #ifndef _WIN32 #define QGA_VIRTIO_PATH_DEFAULT /dev/virtio-ports/org.qemu.guest_agent.0 -#define QGA_STATE_RELATIVE_DIR run +#define QGA_VOLATILE_STATE_RELATIVE_DIR run +#define QGA_STATE_RELATIVE_DIR lib/qemu-ga #define QGA_SERIAL_PATH_DEFAULT /dev/ttyS0 #else #define QGA_VIRTIO_PATH_DEFAULT .\\Global\\org.qemu.guest_agent.0 @@ -121,7 +122,7 @@ init_dfl_pathnames(void) dfl_pathnames.state_dir = qemu_get_local_state_pathname( QGA_STATE_RELATIVE_DIR); dfl_pathnames.pidfile = qemu_get_local_state_pathname( - QGA_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S qemu-ga.pid); + QGA_VOLATILE_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S qemu-ga.pid); } static void quit_handler(int sig) I think this patch is right, but perhaps another hunk should be added. According to commit 39097daf, persistent state should indeed survive guest reboots. This seems appropriate for at least fd_counter. However we store qga.state.isfrozen too in the state directory (state_filepath_isfrozen). According to commit f789aa7b, that file should survive qemu-ga crashes and restarts, but I think it shouldn't survive entire *guest* restarts. (When the guest reboots, its filesystems certainly won't be frozen, and the qga.state.isfrozen will be stale.) What we have now is: - state (including fd_counter and isfrozen status) is lost across guest reboot. This is wrong for fd_counter, but right for isfrozen. If you make the state persistent for real, then: - fd_counter will work more safely, but isfrozen will (theoretically) regress. Hence I think that state_filepath_isfrozen should also use QGA_VOLATILE_STATE_RELATIVE_DIR. And that's a huge mess then, because: - in Windows guests, we only have one state dir, and the state_filepath_isfrozen assignment is currently guest-type independent - in Linux guests, we'd have two state dirs (non-volatile and volatile), but the user can control only one with the -t option. I don't know what to recommend for this. Anyway I have some worries about the 2nd patch, let me continue there. Thanks Laszlo
Re: [Qemu-devel] [PATCH v2 1/1] -machine vmport=off: Allow disabling of VMWare ioport emulation
On Fri, Oct 03, 2014 at 05:33:37PM -0400, Don Slutz wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com
This is a pc q35 only machine opt.
VMWare apparently doesn't like running under QEMU due to our
incomplete emulation of it's special IO Port. This adds a
pc q35 property to allow it to be turned off.
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
Signed-off-by: Don Slutz dsl...@verizon.com
---
hw/i386/pc.c | 19 +++
hw/i386/pc_piix.c| 4 ++--
hw/i386/pc_q35.c | 3 ++-
include/hw/i386/pc.h | 2 ++
qemu-options.hx | 3 +++
vl.c | 4
6 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 82a7daa..8e37a99 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1687,6 +1687,20 @@ static void pc_machine_set_max_ram_below_4g(Object
*obj, Visitor *v,
pcms-max_ram_below_4g = value;
}
+static bool pc_machine_get_vmport(Object *obj, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+return pcms-vmport;
+}
+
+static void pc_machine_set_vmport(Object *obj, bool value, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+pcms-vmport = value;
+}
+
static void pc_machine_initfn(Object *obj)
{
PCMachineState *pcms = PC_MACHINE(obj);
@@ -1699,6 +1713,11 @@ static void pc_machine_initfn(Object *obj)
pc_machine_get_max_ram_below_4g,
pc_machine_set_max_ram_below_4g,
NULL, NULL, NULL);
+pcms-vmport = !xen_enabled();
+object_property_add_bool(obj, PC_MACHINE_VMPORT,
+ pc_machine_get_vmport,
+ pc_machine_set_vmport,
+ NULL);
}
static void pc_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 103d756..03a73ce 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -234,8 +234,8 @@ static void pc_init1(MachineState *machine,
pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
/* init basic PC hardware */
-pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, xen_enabled(),
-0x4);
+pc_basic_device_init(isa_bus, gsi, rtc_state, floppy,
+ !pc_machine-vmport, 0x4);
pc_nic_init(isa_bus, pci_bus);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d4a907c..c5ba93d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -241,7 +241,8 @@ static void pc_q35_init(MachineState *machine)
pc_register_ferr_irq(gsi[13]);
/* init basic PC hardware */
-pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, false, 0xff0104);
+pc_basic_device_init(isa_bus, gsi, rtc_state, floppy,
+ !pc_machine-vmport, 0xff0104);
/* connect pm stuff to lpc */
ich9_lpc_pm_init(lpc);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..96febb9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -35,11 +35,13 @@ struct PCMachineState {
HotplugHandler *acpi_dev;
uint64_t max_ram_below_4g;
+bool vmport;
};
#define PC_MACHINE_ACPI_DEVICE_PROP acpi-device
#define PC_MACHINE_MEMHP_REGION_SIZE hotplug-memory-region-size
#define PC_MACHINE_MAX_RAM_BELOW_4G max-ram-below-4g
+#define PC_MACHINE_VMPORT vmport
/**
* PCMachineClass:
diff --git a/qemu-options.hx b/qemu-options.hx
index 365b56c..fe6b6e5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -33,6 +33,7 @@ DEF(machine, HAS_ARG, QEMU_OPTION_machine, \
property accel=accel1[:accel2[:...]] selects
accelerator\n
supported accelerators are kvm, xen, tcg (default:
tcg)\n
kernel_irqchip=on|off controls accelerated irqchip
support\n
+vmport=on|off controls emulation of vmport (default:
on)\n
kvm_shadow_mem=size of KVM shadow MMU\n
dump-guest-core=on|off include guest memory in a core
dump (default=on)\n
mem-merge=on|off controls memory merge support
(default: on)\n
@@ -51,6 +52,8 @@ than one accelerator specified, the next one is used if the
previous one fails
to initialize.
@item kernel_irqchip=on|off
Enables in-kernel irqchip support for the chosen accelerator when available.
+@item vmport=on|off
+Enables emulation of VMWare IO port, for vmmouse etc. (enabled by default)
@item kvm_shadow_mem=size
Defines the size of the KVM shadow MMU.
@item dump-guest-core=on|off
diff --git a/vl.c b/vl.c
index 9d2aaaf..26fa864 100644
--- a/vl.c
+++ b/vl.c
@@ -389,6 +389,10 @@ static QemuOptsList qemu_machine_opts = {
.name = PC_MACHINE_MAX_RAM_BELOW_4G,
.type = QEMU_OPT_SIZE,
.help = maximum ram below the 4G boundary (32bit
Re: [Qemu-devel] [PATCH 10/11] block: let commit blockjob run in BDS AioContext
On Sat, Oct 04, 2014 at 11:28:22PM +0200, Max Reitz wrote:
On 01.10.2014 19:01, Stefan Hajnoczi wrote:
The commit block job must run in the BlockDriverState AioContext so that
it works with dataplane.
Acquire the AioContext in blockdev.c so starting the block job is safe.
One detail here is that the bdrv_drain_all() must be moved inside the
aio_context_acquire() region so requests cannot sneak in between the
drain and acquire.
Hm, I see the intent, but in patch 5 you said bdrv_drain_all() should never
be called outside of the main loop (at least that's how it appeared to me).
Wouldn't it be enough to use bdrv_drain() on the source BDS, like in patch
9?
There is no contradiction here because qmp_block_commit() is invoked by
the QEMU monitor from the main loop.
The problem with bdrv_drain_all() is that it acquires AioContexts. If
called outside the main loop without taking special care, it could
result in lock ordering problems (e.g. two threads trying to acquire all
AioContexts at the same time while already holding their respective
contexts).
qmp_block_commit() is just traditional QEMU global mutex code so it is
allowed to call bdrv_drain_all().
@@ -140,27 +173,14 @@ wait:
ret = 0;
-if (!block_job_is_cancelled(s-common) sector_num == end) {
-/* success */
-ret = bdrv_drop_intermediate(active, top, base,
s-backing_file_str);
+out:
+if (buf) {
+qemu_vfree(buf);
}
Is this new condition really necessary? However, it won't hurt, so:
This was a mistake. Since commit
94c8ff3a01d9bd1005f066a0ee3fe43c842a43b7 (w32: Make qemu_vfree() accept
NULL like the POSIX implementation) it is no longer necessary to check
for NULL pointers.
You can't teach an old dog new tricks :).
Thanks, will fix in the next revision!
Reviewed-by: Max Reitz mre...@redhat.com
A general question regarding the assertions here and in patch 8: I tried to
break them, but it couldn't find a way. The way I tried was by creating two
devices in different threads with just one qcow2 behind each of them, and
then trying to attach on of those qcow2 BDS to the other as a backing file.
I couldn't find out, how, but I guess this is something we might want to
support in the future. Can we actually be sure that all of the BDS in one
tree are always running in the same AIO context? Are we already enforcing
this?
bdrv_set_aio_context() is recursive so it also sets all the child nodes.
That is the only mechanism to ensure AioContext is consistent across
nodes.
When the BDS graph is manipulated (e.g. attaching new roots, swapping
nodes, etc) we don't perform checks today.
Markus has asked that I add the appropriate assertions so errors are
caught early. I haven't done that yet but it's a good next step.
As far as I'm aware, these patches don't introduce cases where we would
make the AioContext in the graph inconsistent. So I see the AioContext
consistency assertions as a separate patch series (which I will work on
next...hopefully not to discover horrible problems!).
And furthermore, basically all the calls to acquire an AIO context are of
the form aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);. It is *extremely* unlikely if possible
at all, but wouldn't it be possible to change the BDS's AIO context from
another thread after the first function returned and before the lock is
acquired? If that is really the case, I think we should have some atomic
bdrv_acquire_aio_context() function.
No, because only the main loop calls bdrv_set_aio_context(). At the
moment the case you mentioned cannot happen.
Ultimately, we should move away from this only works in the main loop
constraints. In order to provide atomic BDS AioContext acquire we need
a global root that is thread-safe. That doesn't exist today -
bdrv_states is protected by the QEMU global mutex only.
I thought about adding the infrastructure in this patch series but it is
not necessary yet and would make the series more complicated.
The idea is:
* Add bdrv_states_lock to protect the global list of BlockDriverStates
* Integrate bdrv_ref()/bdrv_unref() as well as bdrv_get_aio_context()
so they are atomic and protected by the bdrv_states_lock
So bdrv_find() and other functions that access bdrv_states become the
entry points to acquiring BlockDriverStates in a thread-safe fashion.
bdrv_unref() will need rethinking too to prevent races between freeing a
BDS and bdrv_find().
Can you think of a place where we need this today? I haven't found one
yet but would like one to develop the code against.
Stefan
pgpVoludAPiMl.pgp
Description: PGP signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] target-ppc: Handle ibm, nmi-register RTAS call
On 06.10.14 08:32, Aravinda Prasad wrote:
On Monday 08 September 2014 02:17 AM, Alexander Graf wrote:
On 25.08.14 15:45, Aravinda Prasad wrote:
This patch adds FWNMI support in qemu for powerKVM
guests by handling the ibm,nmi-register rtas call.
Whenever OS issues ibm,nmi-register RTAS call, the
machine check notification address is saved and the
machine check interrupt vector 0x200 is patched to
issue a private hcall.
Signed-off-by: Aravinda Prasad aravi...@linux.vnet.ibm.com
---
hw/ppc/spapr_rtas.c| 91
include/hw/ppc/spapr.h |8
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 02ddbf9..1135d2b 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -277,6 +277,91 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU
*cpu,
rtas_st(rets, 0, ret);
}
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+int i;
+uint32_t branch_inst = 0x4802;
+target_ulong guest_machine_check_addr;
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+/*
+ * Trampoline saves r3 in sprg2 and issues private hcall
+ * to request qemu to build error log. QEMU builds the
+ * error log, copies to rtas-blob and returns the address.
+ * The initial 16 bytes in rtas-blob consists of saved srr0
+ * and srr1 which we restore and pass on the actual error
+ * log address to OS handled mcachine check notification
+ * routine
+ */
+uint32_t trampoline[] = {
+0x7c7243a6,/* mtspr SPRN_SPRG2,r3 */
+0x3860,/* li r3,0 */
+/* 0xf004 is the KVMPPC_H_REPORT_ERR private HCALL */
+0x6063f004,/* ori r3,r3,f004 */
+/* Issue H_CALL */
+0x4422,/* sc 1 */
+0x7c9243a6,/* mtspr r4 sprg2 */
+0xe883,/* ld r4, 0(r3) */
+0x7c9a03a6,/* mtspr r4, srr0 */
+0xe8830008,/* ld r4, 8(r3) */
+0x7c9b03a6,/* mtspr r4, srr1 */
+0x38630010,/* addi r3,r3,16 */
+0x7c9242a6,/* mfspr r4 sprg2 */
+0x4802,/* Branch to address registered
+* by OS. The branch address is
+* patched below */
+0x4800,/* b . */
So how about we just completely change the layout of the RTAS blob?
Imagine something like the following (completely untested):
/ index table /
.long rtas_entry
.long nmi_register
.long nmi_register_final_branch
.long nmi_data
/ RTAS handling code /
.align 1024
rtas_entry:
...
nmi_register:
...
nmi_register_final_branch:
ba .
/ RTAS data regions /
.align 4096
nmi_data:
.long 0
.align 4096
With this we should be able to have a nice hybrid between easily tunable
asm code and an easy to load and handle blob.
Sorry, I was out of office hence could not respond.
Yes, even I prefer something like this.
BTB, did you intend to have this in spapr-rtas.S? The spapr-rtas.S is
compiled into a binary and is read into spapr-rtas_blob. If we want to
have rtas-blob layout something similar to above then we may need to
link the object file of spapr-rtas.S to QEMU so that the symbols in
index table and other places could be resolved inside QEMU.
If this is fine I will include it in v3.
You can't link against the object file with QEMU, since QEMU could be
executed on an x86 machine which can't compile the rtas blob.
In my proposal above, you would maintain a table of entry points at well
known locations in the binary blob (starting from 0 is probably the
easiest).
Alex
Re: [Qemu-devel] Fullscreen Bug with GTK
On Mo, 2014-10-06 at 10:14 +0100, Dr. David Alan Gilbert wrote: * Gerd Hoffmann (kra...@redhat.com) wrote: Hi, There is a bug where the GTK (SDL appears to be broken due to some sdl2 incompatibility, as I understood) menu bar won't hide in fullscreen: SDL-1 support is still there. Are you recommending building against SDL-1 even when SDL-2 is available? (Does that lose you anything?) There are some rough edges still, I havn't flipped the default to SDL-2 yet in upstream qemu because of that. So unless you depend on something new only provided by SDL2 (such as multi-window support) your best bet is building with SDL1 instead. Unfortunately we can't build both and switch at runtime. Patches are accepted too ;) cheers, Gerd
Re: [Qemu-devel] [PATCH 2/2] guest-agent: preserve online/offline state of VCPUs on guest reboot
On 10/06/14 09:44, Igor Mammedov wrote:
Fixes issue when CPU was offlined via libvirt using command:
virsh setvcpus --guest myguest NR_CPUS
but it became onlined again after guest was rebooted.
Fix issue by storing current state of CPUs online state
on persistent storage when GA is being stopped and restore
it when it's started at system boot time.
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
qga/main.c | 102
+
1 file changed, 102 insertions(+)
diff --git a/qga/main.c b/qga/main.c
index 5afba01..1173ca9 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -32,6 +32,7 @@
#include qapi/qmp/dispatch.h
#include qga/channel.h
#include qemu/bswap.h
+#include qga-qmp-commands.h
#ifdef _WIN32
#include qga/service-win32.h
#include qga/vss-win32.h
@@ -57,6 +58,7 @@
#define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR /fsfreeze-hook
#endif
#define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CPU_STATE_GROUP CPU online states
static struct {
const char *state_dir;
@@ -66,6 +68,7 @@ static struct {
typedef struct GAPersistentState {
#define QGA_PSTATE_DEFAULT_FD_COUNTER 1000
int64_t fd_counter;
+GuestLogicalProcessorList *vcpus;
} GAPersistentState;
struct GAState {
@@ -770,6 +773,40 @@ static void
persistent_state_from_keyfile(GAPersistentState *pstate,
pstate-fd_counter =
g_key_file_get_integer(keyfile, global, fd_counter, NULL);
}
+
+if (g_key_file_has_group(keyfile, QGA_CPU_STATE_GROUP)) {
+bool state;
+char **cpu_id_str;
+GuestLogicalProcessor *vcpu;
+GuestLogicalProcessorList *entry;
+GuestLogicalProcessorList *head, **link;
+char **cpus_list = g_key_file_get_keys(keyfile, QGA_CPU_STATE_GROUP,
+ NULL, NULL);
+
+head = NULL;
+link = head;
+for (cpu_id_str = cpus_list; *cpu_id_str; ++cpu_id_str) {
+state = g_key_file_get_boolean(keyfile, QGA_CPU_STATE_GROUP,
+ *cpu_id_str, NULL);
+
+vcpu = g_malloc0(sizeof *vcpu);
+vcpu-logical_id = g_ascii_strtoll(*cpu_id_str, NULL, 0);
+vcpu-online = state;
+vcpu-has_can_offline = true;
+
+entry = g_malloc0(sizeof *entry);
+entry-value = vcpu;
+
+*link = entry;
+link = entry-next;
+}
+pstate-vcpus = head;
+
+g_strfreev(cpus_list);
+}
+if (pstate-vcpus == NULL) {
+pstate-vcpus = qmp_guest_get_vcpus(NULL);
+}
}
static void persistent_state_to_keyfile(const GAPersistentState *pstate,
@@ -779,6 +816,27 @@ static void persistent_state_to_keyfile(const
GAPersistentState *pstate,
g_assert(keyfile);
g_key_file_set_integer(keyfile, global, fd_counter,
pstate-fd_counter);
+
+if (pstate-vcpus) {
+GuestLogicalProcessorList *vcpus = pstate-vcpus;
+
+while (vcpus != NULL) {
+char *logical_id;
+GuestLogicalProcessor *vcpu = vcpus-value;
+
+if (vcpu-can_offline == false) {
+vcpus = vcpus-next;
+continue;
+}
+
+logical_id = g_strdup_printf(% PRId64, vcpu-logical_id);
+g_key_file_set_boolean(keyfile, QGA_CPU_STATE_GROUP,
+ logical_id, vcpu-online);
+g_free(logical_id);
+
+vcpus = vcpus-next;
+}
+}
}
static gboolean write_persistent_state(const GAPersistentState *pstate,
@@ -820,6 +878,47 @@ out:
return ret;
}
+static void ga_clean_vcpu_pstate(GAPersistentState *pstate)
+{
+if (pstate-vcpus) {
+qapi_free_GuestLogicalProcessorList(pstate-vcpus);
+pstate-vcpus = NULL;
+}
+}
+
+static void ga_clean_pstate(GAPersistentState *pstate)
+{
+ga_clean_vcpu_pstate(pstate);
+}
+
+#if defined(__linux__)
+
+static void ga_update_vcpu_pstate(GAPersistentState *pstate, const char
*path)
+{
+Error *local_err = NULL;
+
+ga_clean_vcpu_pstate(pstate);
+pstate-vcpus = qmp_guest_get_vcpus(local_err);
+
+if (local_err != NULL) {
+g_critical(%s, error_get_pretty(local_err));
+error_free(local_err);
+return;
+}
+
+if (!write_persistent_state(pstate, path)) {
+g_critical(failed to commit CPU persistent state to disk);
+}
+}
+
+#else
+
+static void ga_update_vcpu_pstate(GAPersistentState *pstate, const char
*path)
+{
+}
+
+#endif
+
static gboolean read_persistent_state(GAPersistentState *pstate,
const gchar *path, gboolean frozen)
{
@@ -878,6 +977,7 @@ static gboolean read_persistent_state(GAPersistentState
*pstate,
}
persistent_state_from_keyfile(pstate, keyfile);
+
Re: [Qemu-devel] [PATCH v2 0/1] -machine vmport=off: Allow disabling of VMWare ioport emulation
* Don Slutz (dsl...@verizon.com) wrote: Changes v1 to v2 (Don Slutz): make vmport a pc q35 only machine opt I.E. a machine property. Dr. David Alan Gilbert (1): -machine vmport=off: Allow disabling of VMWare ioport emulation Thanks for updating this! Dave hw/i386/pc.c | 19 +++ hw/i386/pc_piix.c| 4 ++-- hw/i386/pc_q35.c | 3 ++- include/hw/i386/pc.h | 2 ++ qemu-options.hx | 3 +++ vl.c | 4 6 files changed, 32 insertions(+), 3 deletions(-) -- 1.8.4 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration
On Fri, Oct 03, 2014 at 02:12:06PM +1000, Alexey Kardashevskiy wrote: BDRV_O_INCOMING is only set when QEMU is about to receive migration and we do not want QEMU to check the file at opening time as there is likely garbage. Is there any other use of BDRV_O_INCOMING? There must be some as bdrv_clear_incoming_migration_all() is called at the end of live migration and I believe there must be a reason for it (cache does not migrate, does it?). BDRV_O_INCOMING is just for live migration. The cached data is not migrated, this is why it must be refreshed upon migration handover. bdrv_invalidate_cache() flushes cache as it could be already initialized even if QEMU is receiving migration - QEMU could have cached some of real disk data. Is that correct? I do not really understand why it would happen if there is BDRV_O_INCOMING set but ok. .bdrv_open() can load metadata from image files (such as the qcow2 L1 tables) and it does this even when BDRV_O_INCOMING is set. That data needs to be re-read at migration handover to prevent the destination QEMU from running with stale image metadata. diff --git a/nbd.c b/nbd.c index e9b539b..953c378 100644 - --- a/nbd.c +++ b/nbd.c @@ -972,6 +972,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, exp-ctx = bdrv_get_aio_context(bs); bdrv_ref(bs); bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, exp); +bdrv_invalidate_cache(bs, NULL); return exp; } Please add a comment to explain why this call is necessary: /* NBD exports are used for non-shared storage migration. Make sure * that BDRV_O_INCOMING is cleared and the image is ready for write * access since the export could be available before migration handover. */ If someone can come up with a 2- or 3-line comment that explains it better, great. The rest of the patch looks like it will work. I'm not thrilled about putting BDRV_O_INCOMING logical inside bdrv_invalidate_cache() because there was no coupling there before, but the code seems correct now. pgpmK5p8VuhKd.pgp Description: PGP signature
Re: [Qemu-devel] Fullscreen Bug with GTK
https://bugs.launchpad.net/qemu/+bug/1294898 A patch was provided by the initial reporter a long time ago. Patch trades one bug for another. Hiding the menu bar also kills the accelerator keys, which is especially problematic for the fullscreen hotkey as there is no way out of fullscreen mode then. How do you come to that conclusion? The reporter explicitly mentioned that removing the Menu disables the Accels and therefore he has provided a patch which attaches the Accels to the Window instead (which, in fact, is how it should be done).
[Qemu-devel] [Bug 1294898] Re: gtk: menubar visible in fullscreen mode with gtk3
For what it's worth, Gerd replied on the list that
Patch trades one bug for another. Hiding the menu bar also kills the
accelerator keys, which is especially problematic for the fullscreen
hotkey as there is no way out of fullscreen mode then.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1294898
Title:
gtk: menubar visible in fullscreen mode with gtk3
Status in QEMU:
New
Bug description:
Using the gtk UI, compiled with gtk3, the menu bar is fully visible in
full screen mode. On gtk2 it's hidden. The set_size_request call isn't
abided on gtk3 it seems.
Simple fix is:
diff --git a/ui/gtk.c b/ui/gtk.c
index 66e886f..7b3bd3d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -805,7 +805,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void
*opaque)
if (!s-full_screen) {
gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s-notebook), FALSE);
-gtk_widget_set_size_request(s-menu_bar, 0, 0);
+gtk_widget_hide(s-menu_bar);
gtk_widget_set_size_request(s-drawing_area, -1, -1);
gtk_window_fullscreen(GTK_WINDOW(s-window));
if (gd_on_vga(s)) {
@@ -815,7 +815,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void
*opaque)
} else {
gtk_window_unfullscreen(GTK_WINDOW(s-window));
gd_menu_show_tabs(GTK_MENU_ITEM(s-show_tabs_item), s);
-gtk_widget_set_size_request(s-menu_bar, -1, -1);
+gtk_widget_show(s-menu_bar);
gtk_widget_set_size_request(s-drawing_area,
surface_width(s-ds),
surface_height(s-ds));
The problem with that is that hiding the menu bar means all its associated
accelerators are no longer usable, so there's way to exit fullscreen mode.
That's kind of a problem :)
We can install the accelerators on the window, but make sure the menu
item still shows the accelerator short cut. Example with the
fullscreen accelerator:
diff --git a/ui/gtk.c b/ui/gtk.c
index 66e886f..fbce2b0 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -799,7 +799,7 @@ static void gd_menu_show_tabs(GtkMenuItem *item, void
*opaque)
}
}
-static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
+static void gd_do_full_screen(void *opaque)
{
GtkDisplayState *s = opaque;
@@ -828,6 +828,11 @@ static void gd_menu_full_screen(GtkMenuItem *item, void
*opaque)
gd_update_cursor(s, FALSE);
}
+static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
+{
+gd_do_full_screen(opaque);
+}
+
static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
{
GtkDisplayState *s = opaque;
@@ -1304,10 +1309,11 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState
*s, GtkAccelGroup *accel_g
gtk_menu_set_accel_group(GTK_MENU(view_menu), accel_group);
s-full_screen_item = gtk_menu_item_new_with_mnemonic(_(_Fullscreen));
-gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s-full_screen_item),
- QEMU/View/Full Screen);
-gtk_accel_map_add_entry(QEMU/View/Full Screen, GDK_KEY_f,
-HOTKEY_MODIFIERS);
+gtk_accel_group_connect(accel_group, GDK_KEY_f, HOTKEY_MODIFIERS, 0,
+ g_cclosure_new_swap(G_CALLBACK(gd_do_full_screen), s, NULL));
+gtk_accel_label_set_accel(
+GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(s-full_screen_item))),
+GDK_KEY_f, HOTKEY_MODIFIERS);
gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s-full_screen_item);
separator = gtk_separator_menu_item_new();
However gtk_accel_label_set_accel, which shows the accel key sequence in the
menu, is gtk 3.8+ :/ So older versions wouldn't have any visual indication of
the shortcuts. Maybe that's not a problem, SDL didn't have any indication of
shortcuts either.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1294898/+subscriptions
Re: [Qemu-devel] Fullscreen Bug with GTK
On Mo, 2014-10-06 at 12:14 +0200, Cedric Sodhi wrote: https://bugs.launchpad.net/qemu/+bug/1294898 A patch was provided by the initial reporter a long time ago. Patch trades one bug for another. Hiding the menu bar also kills the accelerator keys, which is especially problematic for the fullscreen hotkey as there is no way out of fullscreen mode then. How do you come to that conclusion? I've tried to fix that before, without success. The reporter explicitly mentioned that removing the Menu disables the Accels and therefore he has provided a patch which attaches the Accels to the Window instead (which, in fact, is how it should be done). Oh, missed that detail. /me recommends submitting the patches to qemu-devel then (and cc me so I don't miss them). cheers, Gerd
Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] target-ppc: Handle ibm, nmi-register RTAS call
On Monday 06 October 2014 03:10 PM, Alexander Graf wrote:
On 06.10.14 08:32, Aravinda Prasad wrote:
On Monday 08 September 2014 02:17 AM, Alexander Graf wrote:
On 25.08.14 15:45, Aravinda Prasad wrote:
This patch adds FWNMI support in qemu for powerKVM
guests by handling the ibm,nmi-register rtas call.
Whenever OS issues ibm,nmi-register RTAS call, the
machine check notification address is saved and the
machine check interrupt vector 0x200 is patched to
issue a private hcall.
Signed-off-by: Aravinda Prasad aravi...@linux.vnet.ibm.com
---
hw/ppc/spapr_rtas.c| 91
include/hw/ppc/spapr.h |8
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 02ddbf9..1135d2b 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -277,6 +277,91 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU
*cpu,
rtas_st(rets, 0, ret);
}
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+int i;
+uint32_t branch_inst = 0x4802;
+target_ulong guest_machine_check_addr;
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+/*
+ * Trampoline saves r3 in sprg2 and issues private hcall
+ * to request qemu to build error log. QEMU builds the
+ * error log, copies to rtas-blob and returns the address.
+ * The initial 16 bytes in rtas-blob consists of saved srr0
+ * and srr1 which we restore and pass on the actual error
+ * log address to OS handled mcachine check notification
+ * routine
+ */
+uint32_t trampoline[] = {
+0x7c7243a6,/* mtspr SPRN_SPRG2,r3 */
+0x3860,/* li r3,0 */
+/* 0xf004 is the KVMPPC_H_REPORT_ERR private HCALL */
+0x6063f004,/* ori r3,r3,f004 */
+/* Issue H_CALL */
+0x4422,/* sc 1 */
+0x7c9243a6,/* mtspr r4 sprg2 */
+0xe883,/* ld r4, 0(r3) */
+0x7c9a03a6,/* mtspr r4, srr0 */
+0xe8830008,/* ld r4, 8(r3) */
+0x7c9b03a6,/* mtspr r4, srr1 */
+0x38630010,/* addi r3,r3,16 */
+0x7c9242a6,/* mfspr r4 sprg2 */
+0x4802,/* Branch to address registered
+* by OS. The branch address is
+* patched below */
+0x4800,/* b . */
So how about we just completely change the layout of the RTAS blob?
Imagine something like the following (completely untested):
/ index table /
.long rtas_entry
.long nmi_register
.long nmi_register_final_branch
.long nmi_data
/ RTAS handling code /
.align 1024
rtas_entry:
...
nmi_register:
...
nmi_register_final_branch:
ba .
/ RTAS data regions /
.align 4096
nmi_data:
.long 0
.align 4096
With this we should be able to have a nice hybrid between easily tunable
asm code and an easy to load and handle blob.
Sorry, I was out of office hence could not respond.
Yes, even I prefer something like this.
BTB, did you intend to have this in spapr-rtas.S? The spapr-rtas.S is
compiled into a binary and is read into spapr-rtas_blob. If we want to
have rtas-blob layout something similar to above then we may need to
link the object file of spapr-rtas.S to QEMU so that the symbols in
index table and other places could be resolved inside QEMU.
If this is fine I will include it in v3.
You can't link against the object file with QEMU, since QEMU could be
executed on an x86 machine which can't compile the rtas blob.
In my proposal above, you would maintain a table of entry points at well
known locations in the binary blob (starting from 0 is probably the
easiest).
ok. I will include it in v3 of my patch.
Alex
--
Regards,
Aravinda
Re: [Qemu-devel] Fullscreen Bug with GTK
On 6 October 2014 11:28, Gerd Hoffmann kra...@redhat.com wrote: On Mo, 2014-10-06 at 12:14 +0200, Cedric Sodhi wrote: The reporter explicitly mentioned that removing the Menu disables the Accels and therefore he has provided a patch which attaches the Accels to the Window instead (which, in fact, is how it should be done). Oh, missed that detail. /me recommends submitting the patches to qemu-devel then (and cc me so I don't miss them). ...with the appropriate signed-off-by lines, which are missing from the patch in the bug. thanks -- PMM
[Qemu-devel] [PATCH v3 0/2] monitor: add peripheral device del completion support
After inputting device_del command in monitor, we expect to list all hotpluggable devices automatically by pressing tab key. This patchset provides the function to list all peripheral devices such as memory devices. v3: - commit message changes (Igor) - rename function in patch 1 (Igor) - use 'hotpluggable' property to discard non-hotpluggable devices (Igor) v2: - use object_child_foreach() to simplify the implementation (Andreas) Zhu Guihua (2): qdev: add qdev_build_hotpluggable_device_list helper monitor: add del completion for peripheral device hw/core/qdev.c | 14 ++ include/hw/qdev-core.h | 2 ++ monitor.c | 24 3 files changed, 40 insertions(+) -- 1.9.3
[Qemu-devel] [PATCH v3 1/2] qdev: add qdev_build_hotpluggable_device_list helper
For peripheral device del completion, add a function to build a list for
hotpluggable devices.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
---
hw/core/qdev.c | 14 ++
include/hw/qdev-core.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fcb1638..5f4b2b9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -801,6 +801,20 @@ void qdev_alias_all_properties(DeviceState *target, Object
*source)
} while (class != object_class_by_name(TYPE_DEVICE));
}
+int qdev_build_hotpluggable_device_list(Object *obj, void *opaque)
+{
+GSList **list = opaque;
+DeviceState *dev = DEVICE(obj);
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+if (dev-realized dc-hotpluggable) {
+*list = g_slist_append(*list, dev);
+}
+
+object_child_foreach(obj, qdev_build_hotpluggable_device_list, opaque);
+return 0;
+}
+
static bool device_get_realized(Object *obj, Error **errp)
{
DeviceState *dev = DEVICE(obj);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 178fee2..aa76fdc 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -361,6 +361,8 @@ extern int qdev_hotplug;
char *qdev_get_dev_path(DeviceState *dev);
+int qdev_build_hotpluggable_device_list(Object *obj, void *opaque);
+
static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState
*handler,
Error **errp)
{
--
1.9.3
[Qemu-devel] [PATCH v3 2/2] monitor: add del completion for peripheral device
Add peripheral_device_del_completion() to let peripheral device del completion
be possible.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
---
monitor.c | 24
1 file changed, 24 insertions(+)
diff --git a/monitor.c b/monitor.c
index 667efb7..ffe5405 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4351,6 +4351,29 @@ static void device_del_bus_completion(ReadLineState *rs,
BusState *bus,
}
}
+static void peripheral_device_del_completion(ReadLineState *rs,
+ const char *str, size_t len)
+{
+Object *peripheral;
+GSList *list = NULL, *item;
+
+peripheral = object_resolve_path(/machine/peripheral/, NULL);
+
+if (peripheral == NULL) {
+return;
+}
+
+object_child_foreach(peripheral, qdev_build_hotpluggable_device_list,
+ list);
+
+for (item = list; item; item = g_slist_next(item)) {
+DeviceState *dev = item-data;
+if (!strncmp(str, dev-id, len)) {
+readline_add_completion(rs, dev-id);
+}
+}
+}
+
void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str)
{
size_t len;
@@ -4424,6 +4447,7 @@ void device_del_completion(ReadLineState *rs, int
nb_args, const char *str)
len = strlen(str);
readline_set_completion_index(rs, len);
device_del_bus_completion(rs, sysbus_get_default(), str, len);
+peripheral_device_del_completion(rs, str, len);
}
void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
--
1.9.3
Re: [Qemu-devel] [PULL 00/23] Block patches
On 4 October 2014 21:24, Stefan Hajnoczi stefa...@redhat.com wrote: The following changes since commit b00a0ddb31a393b8386d30a9bef4d9bbb249e7ec: Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-20141002-1' into staging (2014-10-02 15:01:48 +0100) are available in the git repository at: git://github.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 767c86d3e752dfc68ff5d018c3b0b63b71b2: blockdev-test: Test device_del after drive_del (2014-10-04 19:28:39 +0100) Applied, thanks. -- PMM
Re: [Qemu-devel] Fullscreen Bug with GTK
Sorry but whom do you expect to sign them off? I'm not a dev. On 6 October 2014 11:28, Gerd Hoffmann kra...@redhat.com wrote: On Mo, 2014-10-06 at 12:14 +0200, Cedric Sodhi wrote: The reporter explicitly mentioned that removing the Menu disables the Accels and therefore he has provided a patch which attaches the Accels to the Window instead (which, in fact, is how it should be done). Oh, missed that detail. /me recommends submitting the patches to qemu-devel then (and cc me so I don't miss them). ...with the appropriate signed-off-by lines, which are missing from the patch in the bug. thanks -- PMM
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
Il 06/10/2014 11:06, Daniel P. Berrange ha scritto: Now this can largely be accomplished by libvirt by simply changing the value of the -vmgenid command line parameter, because most of these scenarios involve the spawning of a new QEMU process. The exception I think is when a running guest is reverted to a previous snapshot, because that is done via a monitor command and not restarting QEMU. So for this VM Generation ID work to be considered complete we need to have a way to dynamically change the VM generation ID on the fly, atomatically with reverting snapshots from the POV of the guest. eg we must load the snapshot state, change the generation ID, and only then start CPUs again. IOW I think this patch series is incomplete wrt the Microsoft spec on generation ID semantics. I think this should not be a problem, as long as there is an idea of how to implement on-the-fly changes to the vmgenid. Gal, do you know how to find back the address of the VM generation ID? Can we put the offset into the DSDT in the migration stream, and then find the ACPI tables in the f-segment at post_load time? Paolo
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
Il 02/10/2014 15:49, Michael S. Tsirkin ha scritto: The issue is that incoming migration might have a different fw_cfg size from what we have. Understood now. I think migrating this value will solve the issue in a cleaner way. Perhaps. The question is whether it would complicate the forwards-migration code beyond what is sane. I think we are practically speaking stuck with RAM. Migrating RAM size is actually useful too, I think someone asked for it. Migrating RAM size was discussed for BIOS and option ROMs, in order to support migration from old versions of QEMU. It was floated around for some time, but ultimately we ended up shipping two copies of affected firmware (128k/256k BIOS, and non-EFI/EFI option ROMs). For BIOS it wouldn't be enough, because the BIOS size affects the memory map. Of course ACPI tables aren't mapped anywhere, but I'd be wary of adding code to migration that is half-broken and almost never used. Also, RAM blocks that have different size would be yet another thing that makes machine types almost compatible with the QEMU version they're supposed to represent. In a scenario similar to John's, with mutable RAM sizes, would have likely broken all machine types, because we would not have bothered doing full backwards-compatibility. I'm not an advocate of backwards bug-compatibility, but I think RAM block sizes are way beyond the line of what we should be allowed to modify between machine types. Paolo
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
Il 06/10/2014 15:52, Michael S. Tsirkin ha scritto: Maybe we should just modify ACPI and rom files in general to use something else, not RAM? It looked like a good fit initially so we went ahead with it, but these things are fairly small, so it's not a problem to migrate them as part of the device state. Yes, that would have been a good design too, but I'm not sure it's worth changing it now. So far, it has certainly helped us; it both revealed bugs (though a bit too late) and kept us honest. These patches would be easy to revert from now till 2.2 release, they're just an incremental improvement on top of 2.1.2. I'll post a v2 that includes the linuxboot changes to look at the memory map, and keep the 160k padding for the sake of old linuxboot option ROMs being migrated to 2.2. Paolo
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
On Mon, Oct 06, 2014 at 03:42:01PM +0200, Paolo Bonzini wrote: Il 02/10/2014 15:49, Michael S. Tsirkin ha scritto: The issue is that incoming migration might have a different fw_cfg size from what we have. Understood now. I think migrating this value will solve the issue in a cleaner way. Perhaps. The question is whether it would complicate the forwards-migration code beyond what is sane. I think we are practically speaking stuck with RAM. Migrating RAM size is actually useful too, I think someone asked for it. Migrating RAM size was discussed for BIOS and option ROMs, in order to support migration from old versions of QEMU. It was floated around for some time, but ultimately we ended up shipping two copies of affected firmware (128k/256k BIOS, and non-EFI/EFI option ROMs). For BIOS it wouldn't be enough, because the BIOS size affects the memory map. Of course ACPI tables aren't mapped anywhere, but I'd be wary of adding code to migration that is half-broken and almost never used. Also, RAM blocks that have different size would be yet another thing that makes machine types almost compatible with the QEMU version they're supposed to represent. In a scenario similar to John's, with mutable RAM sizes, would have likely broken all machine types, because we would not have bothered doing full backwards-compatibility. I'm not an advocate of backwards bug-compatibility, but I think RAM block sizes are way beyond the line of what we should be allowed to modify between machine types. Paolo Maybe we should just modify ACPI and rom files in general to use something else, not RAM? It looked like a good fit initially so we went ahead with it, but these things are fairly small, so it's not a problem to migrate them as part of the device state. -- MST
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
On Mon, Oct 06, 2014 at 03:55:49PM +0200, Paolo Bonzini wrote: Il 06/10/2014 15:52, Michael S. Tsirkin ha scritto: Maybe we should just modify ACPI and rom files in general to use something else, not RAM? It looked like a good fit initially so we went ahead with it, but these things are fairly small, so it's not a problem to migrate them as part of the device state. Yes, that would have been a good design too, but I'm not sure it's worth changing it now. So far, it has certainly helped us; it both revealed bugs (though a bit too late) and kept us honest. Bugs that surface too late is exactly what worries me. I'll try to look at how hard to implement the above change is. These patches would be easy to revert from now till 2.2 release, they're just an incremental improvement on top of 2.1.2. I'll post a v2 that includes the linuxboot changes to look at the memory map, Pls keep linuxboot things a separate patchset. and keep the 160k padding for the sake of old linuxboot option ROMs being migrated to 2.2. Paolo I think we can drop this for 2.2 machine type. We can also drop the fw cfg file for new machine types. -- MST
Re: [Qemu-devel] [PATCH 04/17] mm: gup: make get_user_pages_fast and __get_user_pages_fast latency conscious
Hello, On Fri, Oct 03, 2014 at 11:23:53AM -0700, Linus Torvalds wrote: On Fri, Oct 3, 2014 at 10:07 AM, Andrea Arcangeli aarca...@redhat.com wrote: This teaches gup_fast and __gup_fast to re-enable irqs and cond_resched() if possible every BATCH_PAGES. This is disgusting. Many (most?) __gup_fast() users just want a single page, and the stupid overhead of the multi-page version is already unnecessary. This just makes things much worse. Quite frankly, we should make a single-page version of __gup_fast(), and convert existign users to use that. After that, the few multi-page users could have this extra latency control stuff. Ok. I didn't think at a better way to add the latency control other than to reduce nr_pages in a outer loop instead of altering the inner calls, but this is what I got after implementing it... If somebody has a cleaner way to implement the latency control stuff that's welcome and I'd be glad to replace it. And yes, the single-page version of get_user_pages_fast() is actually latency-critical. shared futexes hit it hard, and yes, I've seen this in profiles. KVM would save a few cycles from a single-page version too. I just thought further optimizations could be added later and this was better than nothing. Considering I've no better idea how to implement the latency control stuff, for now I'll just drop this controversial patch, and I'll convert those get_user_pages to gup_unlocked instead of converting them to gup_fast, which is more than enough to obtain the mmap_sem holding scalability improvement (that also solves the mmap_sem trouble for the userfaultfd). gup_unlocked isn't as good as gup_fast but it's at least better than the current get_user_pages(). I got into this gup_fast latency control stuff purely because there were a few get_user_pages that could have been converted to get_user_pages_fast as they were using current and current-mm as the first two parameters, except for the risk of disabling irq for long. So I tried to do the right thing and fix gup_fast but I'll leave this further optimization queued for later. About the missing commit header for the other patch Paolo already replied to it, to clarify this a bit further in short I expect that FOLL_TRIED flag to be merged through the KVM git tree which already contains it. I'll add a comment to the commit header to specify it. Sorry for the confusion about that patch. Thanks, Andrea
[Qemu-devel] [PATCH] aio / timers: De-document -clock
Commit 6d32717 aio / timers: Remove alarm timers has issues:
1. It silently ignores -clock for backward compatibility.
Incompatible change: -clock help no longer terminates the program.
Tolerable.
2. Failed to update option documentation. In particular, -help still
advises users to try -clock help for available timers. Drop all
documentation on -clock.
3. The 'query-alarm-clock' example in docs/writing-commands.txt no
longer works, and needs to be redone. Can't do that right now, so I
just stick in a FIXME.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
docs/writing-qmp-commands.txt | 2 ++
qemu-options.hx | 12 ++--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 4d86c24..f3df206 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -365,6 +365,8 @@ documentation for information about the other types.
=== User Defined Types ===
+FIXME This example needs to be redone after commit 6d32717
+
For this example we will write the query-alarm-clock command, which returns
information about QEMU's timer alarm. For more information about it, please
check the -clock command-line option.
diff --git a/qemu-options.hx b/qemu-options.hx
index 365b56c..778d0de 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2983,16 +2983,8 @@ Load the contents of @var{file} as an option ROM.
This option is useful to load things like EtherBoot.
ETEXI
-DEF(clock, HAS_ARG, QEMU_OPTION_clock, \
--clock force the use of the given methods for timer alarm.\n \
-To see what timers are available use '-clock help'\n,
-QEMU_ARCH_ALL)
-STEXI
-@item -clock @var{method}
-@findex -clock
-Force the use of the given methods for timer alarm. To see what timers
-are available use @code{-clock help}.
-ETEXI
+HXCOMM Silently ignored for compatibility
+DEF(clock, HAS_ARG, QEMU_OPTION_clock, , QEMU_ARCH_ALL)
HXCOMM Options deprecated by -rtc
DEF(localtime, 0, QEMU_OPTION_localtime, , QEMU_ARCH_ALL)
--
1.9.3
[Qemu-devel] [PATCH] block/curl: Improve type safety of s-timeout.
qemu_opt_get_number returns a uint64_t, and curl_easy_setopt expects a
long (not an int).
Store the timeout (which is a positive number of seconds) as a
uint64_t. Check that the number given by the user is reasonable.
Cast it to long before calling curl_easy_setopt.
Example error message after this change has been applied:
$ ./qemu-img create -f qcow2 /tmp/test.qcow2 \
-b 'json: { file.driver:https,
file.url:https://foo/bar;,
file.timeout:-1 }'
qemu-img: /tmp/test.qcow2: Could not open 'json: { file.driver:https,
file.url:https://foo/bar;, file.timeout:-1 }': timeout parameter is too
large or negative: Invalid argument
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
block/curl.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 225407c..5233ff6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -112,7 +112,7 @@ typedef struct BDRVCURLState {
char *url;
size_t readahead_size;
bool sslverify;
-int timeout;
+uint64_t timeout;
char *cookie;
bool accept_range;
AioContext *aio_context;
@@ -390,7 +390,7 @@ static CURLState *curl_init_state(BlockDriverState *bs,
BDRVCURLState *s)
if (s-cookie) {
curl_easy_setopt(state-curl, CURLOPT_COOKIE, s-cookie);
}
-curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, s-timeout);
+curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, (long)s-timeout);
curl_easy_setopt(state-curl, CURLOPT_WRITEFUNCTION,
(void *)curl_read_cb);
curl_easy_setopt(state-curl, CURLOPT_WRITEDATA, (void *)state);
@@ -546,6 +546,10 @@ static int curl_open(BlockDriverState *bs, QDict *options,
int flags,
s-timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
CURL_TIMEOUT_DEFAULT);
+if (s-timeout 10) {
+error_setg(errp, timeout parameter is too large or negative);
+goto out_noclean;
+}
s-sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
--
2.0.4
[Qemu-devel] [PULL 5/5] translate-all.c: memory walker initial address miscalculation
From: Mikhail Ilyin m.i...@samsung.com
The initial base address is miscalculated in walk_memory_regions().
It has to be shifted TARGET_PAGE_BITS more. Holder variables are
extended to target_ulong size otherwise they don't fit for MIPS N32
(a 32-bit ABI with a 64-bit address space) and qemu won't compile.
The issue led to incorrect debug output of memory maps and a
mis-formed coredumped file.
Signed-off-by: Mikhail Ilyin m.i...@samsung.com
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
include/exec/cpu-all.h | 4 ++--
linux-user/elfload.c | 18 +-
translate-all.c| 33 -
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9d132f..c085804 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -232,8 +232,8 @@ extern uintptr_t qemu_host_page_mask;
#if defined(CONFIG_USER_ONLY)
void page_dump(FILE *f);
-typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
- abi_ulong, unsigned long);
+typedef int (*walk_memory_regions_fn)(void *, target_ulong,
+ target_ulong, unsigned long);
int walk_memory_regions(void *, walk_memory_regions_fn);
int page_get_flags(target_ulong address);
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bea803b..1c04fcf 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2355,9 +2355,9 @@ struct elf_note_info {
};
struct vm_area_struct {
-abi_ulong vma_start; /* start vaddr of memory region */
-abi_ulong vma_end;/* end vaddr of memory region */
-abi_ulong vma_flags; /* protection etc. flags for the region */
+target_ulong vma_start; /* start vaddr of memory region */
+target_ulong vma_end;/* end vaddr of memory region */
+abi_ulong vma_flags; /* protection etc. flags for the region */
QTAILQ_ENTRY(vm_area_struct) vma_link;
};
@@ -2368,13 +2368,13 @@ struct mm_struct {
static struct mm_struct *vma_init(void);
static void vma_delete(struct mm_struct *);
-static int vma_add_mapping(struct mm_struct *, abi_ulong,
- abi_ulong, abi_ulong);
+static int vma_add_mapping(struct mm_struct *, target_ulong,
+ target_ulong, abi_ulong);
static int vma_get_mapping_count(const struct mm_struct *);
static struct vm_area_struct *vma_first(const struct mm_struct *);
static struct vm_area_struct *vma_next(struct vm_area_struct *);
static abi_ulong vma_dump_size(const struct vm_area_struct *);
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags);
static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
@@ -2466,8 +2466,8 @@ static void vma_delete(struct mm_struct *mm)
g_free(mm);
}
-static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
- abi_ulong end, abi_ulong flags)
+static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
+ target_ulong end, abi_ulong flags)
{
struct vm_area_struct *vma;
@@ -2535,7 +2535,7 @@ static abi_ulong vma_dump_size(const struct
vm_area_struct *vma)
return (vma-vma_end - vma-vma_start);
}
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags)
{
struct mm_struct *mm = (struct mm_struct *)priv;
diff --git a/translate-all.c b/translate-all.c
index 2e0265a..ba5c840 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1660,30 +1660,30 @@ void cpu_interrupt(CPUState *cpu, int mask)
struct walk_memory_regions_data {
walk_memory_regions_fn fn;
void *priv;
-uintptr_t start;
+target_ulong start;
int prot;
};
static int walk_memory_regions_end(struct walk_memory_regions_data *data,
- abi_ulong end, int new_prot)
+ target_ulong end, int new_prot)
{
-if (data-start != -1ul) {
+if (data-start != -1u) {
int rc = data-fn(data-priv, data-start, end, data-prot);
if (rc != 0) {
return rc;
}
}
-data-start = (new_prot ? end : -1ul);
+data-start = (new_prot ? end : -1u);
data-prot = new_prot;
return 0;
}
static int walk_memory_regions_1(struct walk_memory_regions_data *data,
- abi_ulong base, int level, void **lp)
+ target_ulong base, int level, void **lp)
{
-abi_ulong pa;
+target_ulong pa;
int i, rc;
if (*lp == NULL) {
@@ -1708,7 +1708,7 @@ static int walk_memory_regions_1(struct
walk_memory_regions_data *data,
void **pp = *lp;
for (i = 0; i V_L2_SIZE; ++i) {
-
[Qemu-devel] [PULL 3/5] linux-user: Simplify timerid checks on g_posix_timers range
From: Alexander Graf ag...@suse.de
We check whether the passed in timer id is negative on all calls
that involve g_posix_timers.
However, these checks are bogus. First off we limit the timer_id to
16 bits which is not what Linux does. Then we check whether it's negative
which it can't be because we masked it.
We can safely remove the masking. For the negativity check we can just
treat the timerid as unsigned and only check for upper boundaries.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/syscall.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index dcb9df9..7087a56 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9615,11 +9615,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
{
/* args: timer_t timerid, int flags, const struct itimerspec
*new_value,
* struct itimerspec * old_value */
-arg1 = 0x;
-if (arg3 == 0 || arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (arg3 == 0 || timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
target_to_host_itimerspec(hspec_new, arg3);
@@ -9635,13 +9636,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_gettime:
{
/* args: timer_t timerid, struct itimerspec *curr_value */
-arg1 = 0x;
+target_ulong timerid = arg1;
+
if (!arg2) {
return -TARGET_EFAULT;
-} else if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+} else if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec;
ret = get_errno(timer_gettime(htimer, hspec));
@@ -9657,11 +9659,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_getoverrun:
{
/* args: timer_t timerid */
-arg1 = 0x;
-if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_getoverrun(htimer));
}
break;
@@ -9672,13 +9675,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_delete:
{
/* args: timer_t timerid */
-arg1 = 0x;
-if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_delete(htimer));
-g_posix_timers[arg1] = 0;
+g_posix_timers[timerid] = 0;
}
break;
}
--
2.0.1
[Qemu-devel] [PULL 4/5] linux-user: don't include timerfd if not needed
From: Riku Voipio riku.voi...@linaro.org Without this, builds on older systems fail with: qemu/linux-user/syscall.c:61:25: warning: sys/timerfd.h: No such file or directory Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 7087a56..e8f1136 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -58,7 +58,9 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include sys/shm.h #include sys/sem.h #include sys/statfs.h +#ifdef CONFIG_TIMERFD #include sys/timerfd.h +#endif #include utime.h #include sys/sysinfo.h //#include sys/user.h -- 2.0.1
[Qemu-devel] [PULL 0/5] linux-user patches for 2.2
From: Riku Voipio riku.voi...@linaro.org The following changes since commit 1831e150606a221898bf46ffaf0453e9952cbbc4: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-09-30 16:45:35 +0100) are available in the git repository at: git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20141006 for you to fetch changes up to 88555b7dfa79d7d21100d0b90730bf43d25d735b: translate-all.c: memory walker initial address miscalculation (2014-10-01 16:16:14 +0300) linux-user pull for 2.2 Clearest linux-user patches sent to the list since august, Apart from Mikhails patch, the rest are quite trivial. Alexander Graf (2): linux-user: Convert blkpg to use a special subop handler linux-user: Simplify timerid checks on g_posix_timers range Mikhail Ilyin (1): translate-all.c: memory walker initial address miscalculation Peter Maydell (1): linux-user: Enable epoll_pwait syscall for ARM Riku Voipio (1): linux-user: don't include timerfd if not needed include/exec/cpu-all.h | 4 ++-- linux-user/arm/syscall_nr.h | 2 +- linux-user/elfload.c| 18 +- linux-user/ioctls.h | 3 ++- linux-user/syscall.c| 85 - linux-user/syscall_types.h | 2 +- translate-all.c | 33 - 7 files changed, 103 insertions(+), 44 deletions(-) -- 2.0.1
[Qemu-devel] [PULL 2/5] linux-user: Convert blkpg to use a special subop handler
From: Alexander Graf ag...@suse.de
The blkpg ioctl can take different payloads depending on the opcode in
its payload structure. Create a new special ioctl handler that can only
deal with partition style ones for now.
This patch fixes running parted for me.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/ioctls.h| 3 ++-
linux-user/syscall.c | 53 ++
linux-user/syscall_types.h | 2 +-
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 609b27c..e672655 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -78,7 +78,8 @@
IOCTL(BLKRAGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKSSZGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKBSZGET, IOC_R, MK_PTR(TYPE_INT))
- IOCTL(BLKPG, IOC_W, MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
+ IOCTL_SPECIAL(BLKPG, IOC_W, do_ioctl_blkpg,
+ MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
#ifdef FIBMAP
IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8fe9df7..dcb9df9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3696,6 +3696,59 @@ out:
return ret;
}
+static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t *buf_temp, int fd,
+ abi_long cmd, abi_long arg)
+{
+void *argptr;
+int target_size;
+const argtype *arg_type = ie-arg_type;
+const argtype part_arg_type[] = { MK_STRUCT(STRUCT_blkpg_partition) };
+abi_long ret;
+
+struct blkpg_ioctl_arg *host_blkpg = (void*)buf_temp;
+struct blkpg_partition host_part;
+
+/* Read and convert blkpg */
+arg_type++;
+target_size = thunk_type_size(arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(buf_temp, argptr, arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+switch (host_blkpg-op) {
+case BLKPG_ADD_PARTITION:
+case BLKPG_DEL_PARTITION:
+/* payload is struct blkpg_partition */
+break;
+default:
+/* Unknown opcode */
+ret = -TARGET_EINVAL;
+goto out;
+}
+
+/* Read and convert blkpg-data */
+arg = (abi_long)(uintptr_t)host_blkpg-data;
+target_size = thunk_type_size(part_arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(host_part, argptr, part_arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+/* Swizzle the data pointer to our local copy and call! */
+host_blkpg-data = host_part;
+ret = get_errno(ioctl(fd, ie-host_cmd, host_blkpg));
+
+out:
+return ret;
+}
+
static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
int fd, abi_long cmd, abi_long arg)
{
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 9d0c92d..1fd4ee0 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -252,4 +252,4 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* op */
TYPE_INT, /* flags */
TYPE_INT, /* datalen */
- MK_PTR(MK_STRUCT(STRUCT_blkpg_partition))) /* data */
+ TYPE_PTRVOID) /* data */
--
2.0.1
[Qemu-devel] [PULL 1/5] linux-user: Enable epoll_pwait syscall for ARM
From: Peter Maydell peter.mayd...@linaro.org We have support for the epoll_pwait syscall, but it wasn't enabled for ARM guests because we hadn't defined the syscall number; correct this deficiency. Reported-by: Dave Flogeras dfloger...@gmail.com Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/arm/syscall_nr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h index bef847c..7d7be7c 100644 --- a/linux-user/arm/syscall_nr.h +++ b/linux-user/arm/syscall_nr.h @@ -350,7 +350,7 @@ #define TARGET_NR_vmsplice (343) #define TARGET_NR_move_pages (344) #define TARGET_NR_getcpu (345) - /* 346 for epoll_pwait */ +#define TARGET_NR_epoll_pwait (346) #define TARGET_NR_kexec_load (347) #define TARGET_NR_utimensat(348) #define TARGET_NR_signalfd (349) -- 2.0.1
Re: [Qemu-devel] [PATCH] block/curl: Improve type safety of s-timeout.
On 10/06/14 16:32, Richard W.M. Jones wrote:
qemu_opt_get_number returns a uint64_t, and curl_easy_setopt expects a
long (not an int).
Store the timeout (which is a positive number of seconds) as a
uint64_t. Check that the number given by the user is reasonable.
Cast it to long before calling curl_easy_setopt.
Example error message after this change has been applied:
$ ./qemu-img create -f qcow2 /tmp/test.qcow2 \
-b 'json: { file.driver:https,
file.url:https://foo/bar;,
file.timeout:-1 }'
qemu-img: /tmp/test.qcow2: Could not open 'json: { file.driver:https,
file.url:https://foo/bar;, file.timeout:-1 }': timeout parameter is too
large or negative: Invalid argument
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
block/curl.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 225407c..5233ff6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -112,7 +112,7 @@ typedef struct BDRVCURLState {
char *url;
size_t readahead_size;
bool sslverify;
-int timeout;
+uint64_t timeout;
char *cookie;
bool accept_range;
AioContext *aio_context;
@@ -390,7 +390,7 @@ static CURLState *curl_init_state(BlockDriverState *bs,
BDRVCURLState *s)
if (s-cookie) {
curl_easy_setopt(state-curl, CURLOPT_COOKIE, s-cookie);
}
-curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, s-timeout);
+curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, (long)s-timeout);
curl_easy_setopt(state-curl, CURLOPT_WRITEFUNCTION,
(void *)curl_read_cb);
curl_easy_setopt(state-curl, CURLOPT_WRITEDATA, (void *)state);
@@ -546,6 +546,10 @@ static int curl_open(BlockDriverState *bs, QDict
*options, int flags,
s-timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
CURL_TIMEOUT_DEFAULT);
+if (s-timeout 10) {
+error_setg(errp, timeout parameter is too large or negative);
+goto out_noclean;
+}
s-sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
Since we're validating s-timeout -- is a zero value okay?
Thanks
Laszlo
Re: [Qemu-devel] [PATCH] block/curl: Improve type safety of s-timeout.
On Mon, Oct 06, 2014 at 04:38:59PM +0200, Laszlo Ersek wrote:
On 10/06/14 16:32, Richard W.M. Jones wrote:
qemu_opt_get_number returns a uint64_t, and curl_easy_setopt expects a
long (not an int).
Store the timeout (which is a positive number of seconds) as a
uint64_t. Check that the number given by the user is reasonable.
Cast it to long before calling curl_easy_setopt.
Example error message after this change has been applied:
$ ./qemu-img create -f qcow2 /tmp/test.qcow2 \
-b 'json: { file.driver:https,
file.url:https://foo/bar;,
file.timeout:-1 }'
qemu-img: /tmp/test.qcow2: Could not open 'json: { file.driver:https,
file.url:https://foo/bar;, file.timeout:-1 }': timeout parameter is
too large or negative: Invalid argument
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
block/curl.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 225407c..5233ff6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -112,7 +112,7 @@ typedef struct BDRVCURLState {
char *url;
size_t readahead_size;
bool sslverify;
-int timeout;
+uint64_t timeout;
char *cookie;
bool accept_range;
AioContext *aio_context;
@@ -390,7 +390,7 @@ static CURLState *curl_init_state(BlockDriverState *bs,
BDRVCURLState *s)
if (s-cookie) {
curl_easy_setopt(state-curl, CURLOPT_COOKIE, s-cookie);
}
-curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, s-timeout);
+curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, (long)s-timeout);
curl_easy_setopt(state-curl, CURLOPT_WRITEFUNCTION,
(void *)curl_read_cb);
curl_easy_setopt(state-curl, CURLOPT_WRITEDATA, (void *)state);
@@ -546,6 +546,10 @@ static int curl_open(BlockDriverState *bs, QDict
*options, int flags,
s-timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
CURL_TIMEOUT_DEFAULT);
+if (s-timeout 10) {
+error_setg(errp, timeout parameter is too large or negative);
+goto out_noclean;
+}
s-sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
Since we're validating s-timeout -- is a zero value okay?
Yes it's OK. It means wait forever:
CURLOPT_TIMEOUT
Pass a long as parameter containing the maximum time in seconds
that you allow the libcurl transfer operation to take. Normally,
name lookups can take a considerable time and limiting opera‐
tions to less than a few minutes risk aborting perfectly normal
operations. This option will cause curl to use the SIGALRM to
enable time-outing system calls.
In unix-like systems, this might cause signals to be used unless
CURLOPT_NOSIGNAL is set.
Default timeout is 0 (zero) which means it never times out.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
Re: [Qemu-devel] [PATCH] block/curl: Improve type safety of s-timeout.
On 10/06/14 16:40, Richard W.M. Jones wrote:
On Mon, Oct 06, 2014 at 04:38:59PM +0200, Laszlo Ersek wrote:
On 10/06/14 16:32, Richard W.M. Jones wrote:
qemu_opt_get_number returns a uint64_t, and curl_easy_setopt expects a
long (not an int).
Store the timeout (which is a positive number of seconds) as a
uint64_t. Check that the number given by the user is reasonable.
Cast it to long before calling curl_easy_setopt.
Example error message after this change has been applied:
$ ./qemu-img create -f qcow2 /tmp/test.qcow2 \
-b 'json: { file.driver:https,
file.url:https://foo/bar;,
file.timeout:-1 }'
qemu-img: /tmp/test.qcow2: Could not open 'json: { file.driver:https,
file.url:https://foo/bar;, file.timeout:-1 }': timeout parameter is
too large or negative: Invalid argument
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
block/curl.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index 225407c..5233ff6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -112,7 +112,7 @@ typedef struct BDRVCURLState {
char *url;
size_t readahead_size;
bool sslverify;
-int timeout;
+uint64_t timeout;
char *cookie;
bool accept_range;
AioContext *aio_context;
@@ -390,7 +390,7 @@ static CURLState *curl_init_state(BlockDriverState *bs,
BDRVCURLState *s)
if (s-cookie) {
curl_easy_setopt(state-curl, CURLOPT_COOKIE, s-cookie);
}
-curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, s-timeout);
+curl_easy_setopt(state-curl, CURLOPT_TIMEOUT, (long)s-timeout);
curl_easy_setopt(state-curl, CURLOPT_WRITEFUNCTION,
(void *)curl_read_cb);
curl_easy_setopt(state-curl, CURLOPT_WRITEDATA, (void *)state);
@@ -546,6 +546,10 @@ static int curl_open(BlockDriverState *bs, QDict
*options, int flags,
s-timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
CURL_TIMEOUT_DEFAULT);
+if (s-timeout 10) {
+error_setg(errp, timeout parameter is too large or negative);
+goto out_noclean;
+}
s-sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
Since we're validating s-timeout -- is a zero value okay?
Yes it's OK. It means wait forever:
CURLOPT_TIMEOUT
Pass a long as parameter containing the maximum time in seconds
that you allow the libcurl transfer operation to take. Normally,
name lookups can take a considerable time and limiting opera‐
tions to less than a few minutes risk aborting perfectly normal
operations. This option will cause curl to use the SIGALRM to
enable time-outing system calls.
In unix-like systems, this might cause signals to be used unless
CURLOPT_NOSIGNAL is set.
Default timeout is 0 (zero) which means it never times out.
Rich.
Reviewed-by: Laszlo Ersek ler...@redhat.com
Re: [Qemu-devel] [PATCH] gdbstub: Allow target CPUs to specify watchpoint STOP_BEFORE_ACCESS flag
On 5 October 2014 23:07, Michael Walle mich...@walle.cc wrote: Am Sonntag, 5. Oktober 2014, 22:48:05 schrieb Peter Maydell: On 5 October 2014 22:36, Peter Maydell peter.mayd...@linaro.org wrote: On 5 October 2014 22:00, Michael Walle mich...@walle.cc wrote: I can confirm that your patch makes qemu stop one instruction earlier. Without your patch the program is stopped at (3). With your patch applied the program is stopped at (2). But I guess the correct point to stop is (1), right? No, gdb wants execution to stop with the PC just after the instruction which issued the memory access, with whatever effects the instruction had having already taken place. So (2) is correct. (I think nicer UI would indeed be to stop at (1) but you can't get that effect on CPUs like x86 which only stop after the wp insn has executed, and they'd rather be consistent.) ...and incidentally the way it achieves this for stop before wp insn CPU targets is that it unsets the watchpoint and automatically steps one instruction before returning control to the gdb user. (You can see this if you turn gdb's remote-protocol debug on.) Ah, now it makes sense :) Tested-by: Michael Walle mich...@walle.cc (for lm32) Thanks to all for review/testing; applied to master. -- PMM
Re: [Qemu-devel] [PATCH v5 01/33] target-arm: increase arrays of registers R13 R14
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
Increasing banked_r13 and banked_r14 to store LR_mon and SP_mon (bank
index 7).
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com
Signed-off-by: Greg Bellows greg.bell...@linaro.org
---
target-arm/cpu.h | 4 ++--
target-arm/machine.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 65a3417..81fffd2 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -153,8 +153,8 @@ typedef struct CPUARMState {
/* Banked registers. */
uint64_t banked_spsr[8];
-uint32_t banked_r13[6];
-uint32_t banked_r14[6];
+uint32_t banked_r13[8];
+uint32_t banked_r14[8];
/* These hold r8-r12. */
uint32_t usr_regs[5];
diff --git a/target-arm/machine.c b/target-arm/machine.c
index ddb7d05..7e69127 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -238,8 +238,8 @@ const VMStateDescription vmstate_arm_cpu = {
},
VMSTATE_UINT32(env.spsr, ARMCPU),
VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
-VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 6),
-VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
+VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 8),
+VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 8),
VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
VMSTATE_UINT64_ARRAY(env.elr_el, ARMCPU, 4),
You need to bump the vmstate version fields if
you change fields like this.
thanks
-- PMM
[Qemu-devel] [PATCH] linuxboot: compute initrd loading address
Even though hw/i386/pc.c tries to compute a valid loading address for the
initrd, close to the top of RAM, this does not take into account other
data that is malloced into that memory by SeaBIOS.
Luckily we can easily look at the memory map to find out how much memory is
used up there. This patch places the initrd in the first four gigabytes,
below the first hole (as returned by INT 15h, AX=e801h).
Without this patch:
[0.00] init_memory_mapping: [mem 0x0700-0x07fd]
[0.00] RAMDISK: [mem 0x0710a000-0x07fd7fff]
With this patch:
[0.00] init_memory_mapping: [mem 0x0700-0x07fd]
[0.00] RAMDISK: [mem 0x07112000-0x07fd]
So linuxboot is able to use the 64k that were added as padding for
QEMU = 2.1.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
pc-bios/linuxboot.bin | Bin 1024 - 1024 bytes
pc-bios/optionrom/linuxboot.S | 47 ++
pc-bios/optionrom/optionrom.h | 21 ---
3 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/pc-bios/linuxboot.bin b/pc-bios/linuxboot.bin
index
e7c36694f997c3c34f7f4af3c2923bd2ef6094e7..130103fb739228a6869aaf1b174b9d20c13378fc
100644
GIT binary patch
delta 168
zcmZqRXyBNj#e9V6V4+-#yc2a7@jn|bXJt}WGMdrBas8gPpg4G+*OE29`Ab?LX5F
zKIeMP(|Cx15y-mOxh}WRz3ZJf7D0oZ-X|7o31)0*E19C!O5XCq~0;uRf+QA1b
zX{7|eo$aa3kRw;nki|IC(Q;0c%?4;T_@=t7IoTF$qbirKj~bOE57or0rk;0)C|f
SJtz7Oyqvi?nJI*kFF^X7ev$m
delta 107
zcmZqRXyBNj#azSGI8k@yWCKP?#+1okj0#LU*e5$O$xYtNXvD|`VlnOD22!$yBUQi
zzh^99+93|DjwV+!H~8~fR%ya{VqY)Kk1)y(snQa0l(6Lo)disUOwEsnkj^F@_gl
G#(w~}wj;0r
diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S
index 748c831..5bc0af0 100644
--- a/pc-bios/optionrom/linuxboot.S
+++ b/pc-bios/optionrom/linuxboot.S
@@ -76,14 +76,45 @@ boot_kernel:
copy_kernel:
+ /* Compute initrd address */
+ mov $0xe801, %ax
+ xor %cx, %cx
+ xor %dx, %dx
+ int $0x15
+
+ /* Output could be in AX/BX or CX/DX */
+ or %cx, %cx
+ jnz 1f
+ or %dx, %dx
+ jnz 1f
+ mov %ax, %cx
+ mov %bx, %dx
+1:
+
+ or %dx, %dx
+ jnz 2f
+ addw$1024, %cx/* add 1 MB */
+ movzwl %cx, %edi
+ shll$10, %edi /* convert to bytes */
+ jmp 3f
+
+2:
+ addw$16777216 16, %dx /* add 16 MB */
+ movzwl %dx, %edi
+ shll$16, %edi /* convert to bytes */
+
+3:
+ read_fw FW_CFG_INITRD_SIZE
+ subl%eax, %edi
+ andl$-4096, %edi /* EDI = start of initrd */
/* We need to load the kernel into memory we can't access in 16 bit
mode, so let's get into 32 bit mode, write the kernel and jump
back again. */
/* Reserve space on the stack for our GDT descriptor. */
- mov %esp, %ebp
- sub $16, %esp
+ mov %esp, %ebp
+ sub $16, %esp
/* Now create the GDT descriptor */
movw$((3 * 8) - 1), -16(%bp)
@@ -108,10 +139,18 @@ copy_kernel:
/* We're now running in 16-bit CS, but 32-bit ES! */
/* Load kernel and initrd */
+ pushl %edi
+ read_fw_blob_addr32_edi(FW_CFG_INITRD)
read_fw_blob_addr32(FW_CFG_KERNEL)
- read_fw_blob_addr32(FW_CFG_INITRD)
read_fw_blob_addr32(FW_CFG_CMDLINE)
- read_fw_blob_addr32(FW_CFG_SETUP)
+
+ read_fw FW_CFG_SETUP_ADDR
+ mov %eax, %edi
+ mov %eax, %ebx
+ read_fw_blob_addr32_edi(FW_CFG_SETUP)
+
+ /* Update the header with the initrd address we chose above */
+ popl%es:0x218(%ebx)
/* And now jump into Linux! */
mov $0, %eax
diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
index ce43608..f1a9021 100644
--- a/pc-bios/optionrom/optionrom.h
+++ b/pc-bios/optionrom/optionrom.h
@@ -51,8 +51,6 @@
.endm
#define read_fw_blob_pre(var) \
- read_fw var ## _ADDR; \
- mov %eax, %edi; \
read_fw var ## _SIZE; \
mov %eax, %ecx; \
mov $var ## _DATA, %ax; \
@@ -68,6 +66,8 @@
* Clobbers: %eax, %edx, %es, %ecx, %edi
*/
#define read_fw_blob(var) \
+ read_fw var ## _ADDR; \
+ mov %eax, %edi; \
read_fw_blob_pre(var); \
/* old as(1) doesn't like this insn so emit the bytes instead: \
rep insb(%dx),
Re: [Qemu-devel] [PATCH] aio / timers: De-document -clock
OK by me - sorry about that.
Alex
On 6 Oct 2014, at 15:19, Markus Armbruster arm...@redhat.com wrote:
Commit 6d32717 aio / timers: Remove alarm timers has issues:
1. It silently ignores -clock for backward compatibility.
Incompatible change: -clock help no longer terminates the program.
Tolerable.
2. Failed to update option documentation. In particular, -help still
advises users to try -clock help for available timers. Drop all
documentation on -clock.
3. The 'query-alarm-clock' example in docs/writing-commands.txt no
longer works, and needs to be redone. Can't do that right now, so I
just stick in a FIXME.
Signed-off-by: Markus Armbruster arm...@redhat.com
Reviewed-by: Alex Bligh a...@alex.org.uk
---
docs/writing-qmp-commands.txt | 2 ++
qemu-options.hx | 12 ++--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 4d86c24..f3df206 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -365,6 +365,8 @@ documentation for information about the other types.
=== User Defined Types ===
+FIXME This example needs to be redone after commit 6d32717
+
For this example we will write the query-alarm-clock command, which returns
information about QEMU's timer alarm. For more information about it, please
check the -clock command-line option.
diff --git a/qemu-options.hx b/qemu-options.hx
index 365b56c..778d0de 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2983,16 +2983,8 @@ Load the contents of @var{file} as an option ROM.
This option is useful to load things like EtherBoot.
ETEXI
-DEF(clock, HAS_ARG, QEMU_OPTION_clock, \
--clock force the use of the given methods for timer alarm.\n \
-To see what timers are available use '-clock help'\n,
-QEMU_ARCH_ALL)
-STEXI
-@item -clock @var{method}
-@findex -clock
-Force the use of the given methods for timer alarm. To see what timers
-are available use @code{-clock help}.
-ETEXI
+HXCOMM Silently ignored for compatibility
+DEF(clock, HAS_ARG, QEMU_OPTION_clock, , QEMU_ARCH_ALL)
HXCOMM Options deprecated by -rtc
DEF(localtime, 0, QEMU_OPTION_localtime, , QEMU_ARCH_ALL)
--
1.9.3
--
Alex Bligh
[Qemu-devel] [RFC PATCH] target-i386: move generic memory hotplug methods to DSDTs
This makes it simpler to keep the SSDT byte-for-byte identical for a
given machine type, which is a goal we want to have for 2.2 and newer
types.
This is not tested well and is still missing update of make check
data, but I wanted to throw this out for an early look.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i386/acpi-dsdt-mem-hotplug.dsl | 176
hw/i386/acpi-dsdt.dsl | 3 +-
hw/i386/acpi-dsdt.hex.generated | 795 ++-
hw/i386/q35-acpi-dsdt.dsl | 3 +-
hw/i386/q35-acpi-dsdt.hex.generated | 797 +++-
hw/i386/ssdt-mem.hex.generated | 8 +-
hw/i386/ssdt-misc.dsl | 156 ---
hw/i386/ssdt-misc.hex.generated | 787 +--
8 files changed, 1773 insertions(+), 952 deletions(-)
create mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl
diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl
b/hw/i386/acpi-dsdt-mem-hotplug.dsl
new file mode 100644
index 000..2a36c47
--- /dev/null
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -0,0 +1,176 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see http://www.gnu.org/licenses/.
+ */
+
+External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
+
+Scope(\_SB.PCI0) {
+Device(MEMORY_HOTPLUG_DEVICE) {
+Name(_HID, PNP0A06)
+Name(_UID, Memory hotplug resources)
+External(MEMORY_SLOTS_NUMBER, IntObj)
+
+/* Memory hotplug IO registers */
+OperationRegion(MEMORY_HOTPLUG_IO_REGION, SystemIO,
+ACPI_MEMORY_HOTPLUG_BASE,
+ACPI_MEMORY_HOTPLUG_IO_LEN)
+
+Name(_CRS, ResourceTemplate() {
+IO(Decode16, ACPI_MEMORY_HOTPLUG_BASE,
ACPI_MEMORY_HOTPLUG_BASE,
+ 0, ACPI_MEMORY_HOTPLUG_IO_LEN, IO)
+})
+
+Method(_STA, 0) {
+If (LEqual(MEMORY_SLOTS_NUMBER, Zero)) {
+Return(0x0)
+}
+/* present, functioning, decoding, not shown in UI */
+Return(0xB)
+}
+
+Field(MEMORY_HOTPLUG_IO_REGION, DWordAcc, NoLock, Preserve) {
+MEMORY_SLOT_ADDR_LOW, 32, // read only
+MEMORY_SLOT_ADDR_HIGH, 32, // read only
+MEMORY_SLOT_SIZE_LOW, 32, // read only
+MEMORY_SLOT_SIZE_HIGH, 32, // read only
+MEMORY_SLOT_PROXIMITY, 32, // read only
+}
+Field(MEMORY_HOTPLUG_IO_REGION, ByteAcc, NoLock, Preserve) {
+Offset(20),
+MEMORY_SLOT_ENABLED, 1, // 1 if enabled, read only
+MEMORY_SLOT_INSERT_EVENT, 1, // (read) 1 if has a insert
event. (write) 1 to clear event
+}
+
+Mutex (MEMORY_SLOT_LOCK, 0)
+Field (MEMORY_HOTPLUG_IO_REGION, DWordAcc, NoLock, Preserve) {
+MEMORY_SLOT_SLECTOR, 32, // DIMM selector, write only
+MEMORY_SLOT_OST_EVENT, 32, // _OST event code, write only
+MEMORY_SLOT_OST_STATUS, 32, // _OST status code, write only
+}
+
+Method(MEMORY_SLOT_SCAN_METHOD, 0) {
+If (LEqual(MEMORY_SLOTS_NUMBER, Zero)) {
+ Return(Zero)
+}
+
+Store(Zero, Local0) // Mem devs iterrator
+Acquire(MEMORY_SLOT_LOCK, 0x)
+while (LLess(Local0, MEMORY_SLOTS_NUMBER)) {
+Store(Local0, MEMORY_SLOT_SLECTOR) // select Local0 DIMM
+If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory
device needs check
+MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
+Store(1, MEMORY_SLOT_INSERT_EVENT)
+}
+// TODO: handle memory eject request
+Add(Local0, One, Local0) // goto next DIMM
+}
+Release(MEMORY_SLOT_LOCK)
+Return(One)
+}
+
+Method(MEMORY_SLOT_STATUS_METHOD, 1) {
+Store(Zero, Local0)
+
+Acquire(MEMORY_SLOT_LOCK, 0x)
+Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
+
+If (LEqual(MEMORY_SLOT_ENABLED, One)) {
+Store(0xF, Local0)
+}
+
+
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
On Wed, 17 Sep 2014 20:39:25 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Sep 17, 2014 at 07:21:09PM +0200, Greg Kurz wrote:
On Sun, 14 Sep 2014 21:30:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:
Current support for bus master (clearing OK bit)
together with the need to support guests which do not
enable PCI bus mastering, leads to extra state in
VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust
in case of cross-version migration for the case when
guests use the device before setting DRIVER_OK.
Rip out VIRTIO_PCI_FLAG_BUS_MASTER_BUG and implement a simpler
work-around: treat clearing of PCI_COMMAND as a virtio reset. Old
guests never touch this bit so they will work.
As reset clears device status, DRIVER and MASTER bits are
now in sync, so we can fix up cross-version migration simply
by synchronising them, without need to detect a buggy guest
explicitly.
Drop tracking VIRTIO_PCI_FLAG_BUS_MASTER_BUG completely.
As reset makes the device quiescent, in the future we'll be able to drop
checking OK bit in a bunch of places.
Cc: Jason Wang jasow...@redhat.com
Cc: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Hi Michael,
I am not quite sure how to test this patch with my pseries based setup...
Migrating from qemu-2.1 to qemu-master ?
Cheers.
--
Greg
Exactly. And back! Pls don't forget to specify the 2.1 machine type.
Thanks!
Michael,
Nikunj and I had started to investigate the pseries breakage: the QEMU
originated reset brought by this patch clears the vq and breaks SLOF.
This isn't a surprise since reset should always come from the driver,
not the device.
Since commit 45363e46aeebfc99753389649eac7c7fc22bfe52 has reverted this
patch, QEMU works again for pseries and virtio. :)
So back to the initial issue, I've tried to migrate a pseries-2.1 guest running
rhel65, from QEMU v2.1.2 to QEMU master, back and forth, several times and it
always succeeded... what symptom this patch was expected to fix ?
Cheers.
--
Greg
hw/virtio/virtio-pci.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a827cd4..f560814 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
-/* Flags track per-device state like workarounds for quirks in older
guests. */
-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 0)
-
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
VirtIOPCIProxy *dev);
@@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque,
uint32_t addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
}
-
-/* Linux before 2.6.34 sets the device as OK without enabling
- the PCI device bus master bit. In this case we need to disable
- some safety checks. */
-if ((val VIRTIO_CONFIG_S_DRIVER_OK)
-!(proxy-pci_dev.config[PCI_COMMAND] PCI_COMMAND_MASTER)) {
-proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
-}
break;
case VIRTIO_MSI_CONFIG_VECTOR:
msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
@@ -480,13 +469,18 @@ static void virtio_write_config(PCIDevice *pci_dev,
uint32_t address,
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+uint8_t cmd = proxy-pci_dev.config[PCI_COMMAND];
+
pci_default_write_config(pci_dev, address, val, len);
if (range_covers_byte(address, len, PCI_COMMAND)
!(pci_dev-config[PCI_COMMAND] PCI_COMMAND_MASTER)
-!(proxy-flags VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
+(cmd PCI_COMMAND_MASTER)) {
+/* Bus driver disables bus mastering - make it act
+ * as a kind of reset to render the device quiescent. */
virtio_pci_stop_ioeventfd(proxy);
-virtio_set_status(vdev, vdev-status
~VIRTIO_CONFIG_S_DRIVER_OK);
+virtio_reset(vdev);
+msix_unuse_all_vectors(proxy-pci_dev);
}
}
@@ -895,11 +889,19 @@ static void virtio_pci_vmstate_change(DeviceState
*d, bool running)
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
if (running) {
-/* Try to find out if the guest has bus master disabled, but is
- in ready state. Then we have a buggy guest OS. */
-if ((vdev-status VIRTIO_CONFIG_S_DRIVER_OK)
-
[Qemu-devel] [PATCH v2 0/3] Migration-safe ACPI table sizing algorithm
In the emergency last-minute patches of QEMU 2.1 we did two things: - fixed migration problems from 1.7 or 2.0 to 2.1 due to changes in ACPI table sizes - ensured that future versions will not break migration compatibility with 2.2 for reasonable configurations (with ACPI tables smaller than a hundred kilobytes, roughly) However, this came at the cost of wasting 128 KB unconditionally on even the smaller configuration, and we didn't provide a mechanism to ensure compatibility with larger configurations. This series provides this mechanism. As mentioned early, the design is to consider the SSDT immutable and versioned (together with other non-AML tables such as HPET, TPMA and MADT, SRAT, MCFG, DMAR). The DSDT instead can change more or less arbitrarily. To do this, we add padding after the DSDT to allow for future growth (patch 1). Once we do this, the size of the ACPI table fw_cfg file is constant given a machine type and a command-line, so we do not need anymore the larger 128KB padding (patch 2). Patch 3 is just cleanups. Paolo v1-v2: drop linuxboot changes, instead modify the option ROM in a separate patch Paolo Bonzini (3): pc: introduce new ACPI table sizing algorithm pc: go back to smaller ACPI tables pc: clean up pre-2.1 compatibility code hw/i386/acpi-build.c | 20 +++- hw/i386/pc_piix.c| 20 +++- hw/i386/pc_q35.c | 6 -- include/hw/i386/pc.h | 2 ++ 4 files changed, 36 insertions(+), 12 deletions(-) -- 2.1.0
[Qemu-devel] [PATCH v2 1/3] pc: introduce new ACPI table sizing algorithm
Add padding after the DSDT. Tables that vary depending on the
command-line arguments will have to be byte-equivalent across QEMU
versions = 2.2, while fixed tables (including the DSDT) can be
changed freely.
This new algorithm will let us present smaller ACPI blobs to
the guest, which avoids bugs with -kernel/-initrd and 32-bit
RHEL5 guests. However, this patch does not change the size of
the blobs yet; for now, the values of the parameters are tuned
to have 2.1-compatible sizes.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i386/acpi-build.c | 11 +++
hw/i386/pc_piix.c| 5 +
include/hw/i386/pc.h | 2 ++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a313321..6bffc75 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -65,8 +65,6 @@
#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
#define ACPI_BUILD_ALIGN_SIZE 0x1000
-#define ACPI_BUILD_TABLE_SIZE 0x2
-
typedef struct AcpiCpuInfo {
DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
} AcpiCpuInfo;
@@ -1597,6 +1595,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables
*tables)
acpi_add_table(table_offsets, tables-table_data);
build_fadt(tables-table_data, tables-linker, pm, facs, dsdt);
+if (guest_info-fixed_table_align) {
+acpi_align_size(tables-table_data, guest_info-fixed_table_align);
+}
+
ssdt = tables-table_data-len;
acpi_add_table(table_offsets, tables-table_data);
build_ssdt(tables-table_data, tables-linker, cpu, pm, misc, pci,
@@ -1681,14 +1683,15 @@ void acpi_build(PcGuestInfo *guest_info,
AcpiBuildTables *tables)
g_array_set_size(tables-table_data, legacy_table_size);
} else {
/* Make sure we have a buffer in case we need to resize the tables. */
-if (tables-table_data-len ACPI_BUILD_TABLE_SIZE / 2) {
+if (!guest_info-fixed_table_align
+ tables-table_data-len guest_info-acpi_table_align / 2) {
/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots.
*/
error_report(Warning: ACPI tables are larger than 64k.);
error_report(Warning: migration may not work.);
error_report(Warning: please remove CPUs, NUMA nodes,
memory slots or PCI bridges.);
}
-acpi_align_size(tables-table_data, ACPI_BUILD_TABLE_SIZE);
+acpi_align_size(tables-table_data, guest_info-acpi_table_align);
}
acpi_align_size(tables-linker, ACPI_BUILD_ALIGN_SIZE);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 103d756..060f6ec 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,8 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_acpi_build = true;
static int legacy_acpi_table_size;
+static int fixed_table_align = 0;
+static int acpi_table_align = 131072;
static bool smbios_defaults = true;
static bool smbios_legacy_mode;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
@@ -164,6 +166,8 @@ static void pc_init1(MachineState *machine,
guest_info-has_acpi_build = has_acpi_build;
guest_info-legacy_acpi_table_size = legacy_acpi_table_size;
+guest_info-fixed_table_align = fixed_table_align;
+guest_info-acpi_table_align = acpi_table_align;
guest_info-isapc_ram_fw = !pci_enabled;
guest_info-has_reserved_memory = has_reserved_memory;
@@ -321,6 +325,7 @@ static void pc_compat_2_0(MachineState *machine)
* QEMU 1.7 it is 6414. For RHEL/CentOS 7.0 it is 6418.
*/
legacy_acpi_table_size = 6652;
+acpi_table_align = 4096;
smbios_legacy_mode = true;
has_reserved_memory = false;
pc_set_legacy_acpi_data_size();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..517e729 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -94,6 +94,8 @@ struct PcGuestInfo {
uint64_t *node_cpu;
FWCfgState *fw_cfg;
int legacy_acpi_table_size;
+int fixed_table_align;
+int acpi_table_align;
bool has_acpi_build;
bool has_reserved_memory;
};
--
2.1.0
[Qemu-devel] [PATCH v2 2/3] pc: go back to smaller ACPI tables
The new algorithm introduced by the previous patch lets us make tables
smaller and avoid bugs due to large tables.
Use it for 2.2+ machine types by tweaking the default fixed_table_align
and acpi_table_align values. At the same time, preserve backwards-compatible
logic for pc-i440fx-2.1.
Without this patch:
[0.00] BIOS-e820: [mem 0x0010-0x07fd] usable
[0.00] BIOS-e820: [mem 0x07fe-0x07ff] reserved
...
[0.00] init_memory_mapping: [mem 0x0700-0x07fd] usable
[0.00] RAMDISK: [mem 0x07112000-0x07fd]
With this patch:
[0.00] BIOS-e820: [mem 0x0010-0x07ffafff] usable
[0.00] BIOS-e820: [mem 0x07ffb000-0x07ff] reserved
...
[0.00] init_memory_mapping: [mem 0x0700-0x07ffafff]
[0.00] RAMDISK: [mem 0x07122000-0x07fe]
Thanks to the new linuxboot option ROM, the initrd is loaded 64k above.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i386/pc_piix.c | 19 ---
hw/i386/pc_q35.c | 6 --
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 060f6ec..0d3a191 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,8 +61,8 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_acpi_build = true;
static int legacy_acpi_table_size;
-static int fixed_table_align = 0;
-static int acpi_table_align = 131072;
+static int fixed_table_align = 16384;
+static int acpi_table_align = 4096;
static bool smbios_defaults = true;
static bool smbios_legacy_mode;
/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
@@ -306,6 +306,12 @@ static void pc_init_pci(MachineState *machine)
pc_init1(machine, 1, 1);
}
+static void pc_compat_2_1(MachineState *machine)
+{
+fixed_table_align = 0;
+acpi_table_align = 131072;
+}
+
static void pc_compat_2_0(MachineState *machine)
{
/* This value depends on the actual DSDT and SSDT compiled into
@@ -324,6 +330,7 @@ static void pc_compat_2_0(MachineState *machine)
* 6652 is valid for QEMU 2.0, the right value for pc-i440fx-1.7 on
* QEMU 1.7 it is 6414. For RHEL/CentOS 7.0 it is 6418.
*/
+pc_compat_2_1(machine);
legacy_acpi_table_size = 6652;
acpi_table_align = 4096;
smbios_legacy_mode = true;
@@ -373,6 +380,12 @@ static void pc_compat_1_2(MachineState *machine)
x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
}
+static void pc_init_pci_2_1(MachineState *machine)
+{
+pc_compat_2_1(machine);
+pc_init_pci(machine);
+}
+
static void pc_init_pci_2_0(MachineState *machine)
{
pc_compat_2_0(machine);
@@ -476,7 +489,7 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
static QEMUMachine pc_i440fx_machine_v2_1 = {
PC_I440FX_2_1_MACHINE_OPTIONS,
.name = pc-i440fx-2.1,
-.init = pc_init_pci,
+.init = pc_init_pci_2_1,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_2_1,
{ /* end of list */ }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d4a907c..3443f32 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -153,10 +153,12 @@ static void pc_q35_init(MachineState *machine)
guest_info-has_acpi_build = has_acpi_build;
guest_info-has_reserved_memory = has_reserved_memory;
-/* Migration was not supported in 2.0 for Q35, so do not bother
- * with this hack (see hw/i386/acpi-build.c).
+/* Migration was not supported in 2.0 for Q35, so do not bother with
+ * hacks around the ACPI table size (see hw/i386/acpi-build.c).
*/
guest_info-legacy_acpi_table_size = 0;
+guest_info-fixed_table_align = 16384;
+guest_info-acpi_table_align = 4096;
if (smbios_defaults) {
MachineClass *mc = MACHINE_GET_CLASS(machine);
--
2.1.0
Re: [Qemu-devel] [PATCH v5 02/33] target-arm: add arm_is_secure() function
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
arm_is_secure() function allows to determine CPU security state
if the CPU implements Security Extensions/EL3.
arm_is_secure_below_el3() returns true if CPU is in secure state
below EL3.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
---
target-arm/cpu.h | 38 ++
1 file changed, 38 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 81fffd2..10afef0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -753,6 +753,44 @@ static inline int arm_feature(CPUARMState *env, int
feature)
return (env-features (1ULL feature)) != 0;
}
+
+/* Return true if exception level below EL3 is in secure state */
+static inline bool arm_is_secure_below_el3(CPUARMState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+return !(env-cp15.scr_el3 SCR_NS);
+} else if (arm_feature(env, ARM_FEATURE_EL2)) {
+return false;
+} else {
+/* IMPDEF: QEMU defaults to non-secure */
+return false;
I would be happy to fold both these identical 'return false'
cases together and have a comment that it's only IMPDEF
if EL2 isn't implemented.
+}
+#else
+return false;
+#endif
+}
+
+/* Return true if the processor is in secure state */
+static inline bool arm_is_secure(CPUARMState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+if (env-aarch64 extract32(env-pstate, 2, 2) == 3) {
+/* CPU currently in Aarch64 state and EL3 */
Nit: AArch64 with two capital 'A's (here and elsewhere).
+return true;
+} else if (!env-aarch64
+(env-uncached_cpsr CPSR_M) == ARM_CPU_MODE_MON) {
+/* CPU currently in Aarch32 state and monitor mode */
+return true;
+}
+}
+return arm_is_secure_below_el3(env);
+#else
+return false;
+#endif
+}
I checked your git tree and we don't actually use
arm_is_secure_below_el3() anywhere except in
arm_is_secure(), do we? That suggests to me we should
just fold the two functions together.
Can these functions live in internals.h rather than cpu.h?
(The difference is that internals.h is restricted to only
target-arm/ code whereas cpu.h is auto-included for a much
wider set of files.)
thanks
-- PMM
[Qemu-devel] [PATCH v2 3/3] pc: clean up pre-2.1 compatibility code
Now that the alignment is parameterized, we can share the call to
acpi_align_size between all three (1.7-2.0/2.1/2.2+) sizing algorithms.
Also, with the new rule that SSDT cannot change except with
machine-type compat code, the magic 97 constant for a CPU's
AML size is not anymore legacy, so rename it.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i386/acpi-build.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6bffc75..ec6828e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -62,8 +62,8 @@
* a little bit, there should be plenty of free space since the DSDT
* shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
*/
-#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
-#define ACPI_BUILD_ALIGN_SIZE 0x1000
+#define ACPI_BUILD_CPU_AML_SIZE97
+#define ACPI_BUILD_ALIGN_SIZE 0x1000
typedef struct AcpiCpuInfo {
DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
@@ -1672,10 +1672,9 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables
*tables)
*/
int legacy_aml_len =
guest_info-legacy_acpi_table_size +
-ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus;
+ACPI_BUILD_CPU_AML_SIZE * max_cpus;
int legacy_table_size =
-ROUND_UP(tables-table_data-len - aml_len + legacy_aml_len,
- ACPI_BUILD_ALIGN_SIZE);
+tables-table_data-len - aml_len + legacy_aml_len;
if (tables-table_data-len legacy_table_size) {
/* Should happen only with PCI bridges and -M pc-i440fx-2.0. */
error_report(Warning: migration may not work.);
@@ -1691,8 +1690,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables
*tables)
error_report(Warning: please remove CPUs, NUMA nodes,
memory slots or PCI bridges.);
}
-acpi_align_size(tables-table_data, guest_info-acpi_table_align);
}
+acpi_align_size(tables-table_data, guest_info-acpi_table_align);
acpi_align_size(tables-linker, ACPI_BUILD_ALIGN_SIZE);
--
2.1.0
Re: [Qemu-devel] [PULL 0/5] linux-user patches for 2.2
On 6 October 2014 15:34, riku.voi...@linaro.org wrote: From: Riku Voipio riku.voi...@linaro.org The following changes since commit 1831e150606a221898bf46ffaf0453e9952cbbc4: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-09-30 16:45:35 +0100) are available in the git repository at: git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20141006 for you to fetch changes up to 88555b7dfa79d7d21100d0b90730bf43d25d735b: translate-all.c: memory walker initial address miscalculation (2014-10-01 16:16:14 +0300) linux-user pull for 2.2 Clearest linux-user patches sent to the list since august, Apart from Mikhails patch, the rest are quite trivial. Alexander Graf (2): linux-user: Convert blkpg to use a special subop handler linux-user: Simplify timerid checks on g_posix_timers range Mikhail Ilyin (1): translate-all.c: memory walker initial address miscalculation Peter Maydell (1): linux-user: Enable epoll_pwait syscall for ARM Riku Voipio (1): linux-user: don't include timerfd if not needed Hi. I'm afraid this doesn't compile on my ARM box: /root/qemu/linux-user/syscall.c: In function ‘do_syscall’: /root/qemu/linux-user/syscall.c:9695:9: error: implicit declaration of function ‘timerfd_create’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9695:9: error: nested extern declaration of ‘timerfd_create’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9705:13: error: implicit declaration of function ‘timerfd_gettime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9705:13: error: nested extern declaration of ‘timerfd_gettime’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9728:13: error: implicit declaration of function ‘timerfd_settime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9728:13: error: nested extern declaration of ‘timerfd_settime’ [-Werror=nested-externs] cc1: all warnings being treated as errors thanks -- PMM
Re: [Qemu-devel] [PATCH v5 03/33] target-arm: reject switching to monitor mode
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: From: Sergey Fedorov s.fedo...@samsung.com ...from non-secure state. Signed-off-by: Sergey Fedorov s.fedo...@samsung.com Signed-off-by: Fabian Aggeler aggel...@ethz.ch Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Greg Bellows greg.bell...@linaro.org Reviewed-by: Peter Maydell peter.mayd...@linaro.org though as a style nit I would prefer the commit to read: === Subject: target-arm: reject switching to monitor mode if NS Reject switching to monitor mode from non-secure state. === since the oneline summary should stand on its own. PS: please drop Sergey's now-bouncing Samsung address from the CC list when you send v6 of this series :-) --- target-arm/helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index 2669e15..d6e3b52 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3531,6 +3531,8 @@ static int bad_mode_switch(CPUARMState *env, int mode) case ARM_CPU_MODE_IRQ: case ARM_CPU_MODE_FIQ: return 0; +case ARM_CPU_MODE_MON: +return !arm_is_secure(env); default: return 1; } -- 1.8.3.2 thanks -- PMM
Re: [Qemu-devel] [PATCH] gdb: provide the name of the architecture in the target.xml
On Tue, 30 Sep 2014 17:23:47 +0200 Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com Acked-by: Cornelia Huck cornelia.h...@de.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Reviewed-by: Andreas Färber afaer...@suse.de Cc: Andrzej Zaborowski balr...@gmail.com Cc: Peter Maydell peter.mayd...@linaro.org Cc: Vassili Karpov (malc) av1...@comtv.ru CC: Edgar Iglesias edgar.igles...@gmail.com CC: Richard Henderson r...@twiddle.net --- gdbstub.c | 19 --- include/qom/cpu.h | 2 ++ target-arm/cpu64.c | 1 + target-ppc/translate_init.c | 2 ++ target-s390x/cpu.c | 1 + 5 files changed, 18 insertions(+), 7 deletions(-) I will send this with the next pile of s390x updates, unless someone on cc: has any objections.
Re: [Qemu-devel] [PATCH v5 04/33] target-arm: rename arm_current_pl to arm_current_el
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: Renamed the arm_current_pl CPU function to more accurately represent that it returns the ARMv8 EL rather than ARMv7 PL. Signed-off-by: Greg Bellows greg.bell...@linaro.org --- target-arm/cpu.h | 18 +- target-arm/helper-a64.c| 6 +++--- target-arm/helper.c| 22 +++--- target-arm/internals.h | 2 +- target-arm/op_helper.c | 16 target-arm/translate-a64.c | 2 +- target-arm/translate.c | 2 +- 7 files changed, 34 insertions(+), 34 deletions(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 10afef0..101d139 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -982,7 +982,7 @@ static inline bool cptype_valid(int cptype) #define PL1_RW (PL1_R | PL1_W) #define PL0_RW (PL0_R | PL0_W) -static inline int arm_current_pl(CPUARMState *env) +static inline int arm_current_el(CPUARMState *env) I suggest we add a brief comment before the function: /* Return the current Exception Level (as per ARMv8; * note that this differs from the ARMv7 Privilege Level). */ You should also fix the PL2 and PL3 references in the comments to read EL2 and EL3. thanks -- PMM
Re: [Qemu-devel] [PATCH v5 05/33] target-arm: make arm_current_pl() return PL3
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
Make arm_current_pl() return PL3 for secure PL1 and monitor mode.
Increase MMU modes since mmu_index is directly infered from arm_
current_pl(). Changes assertion in arm_el_is_aa64() to allow EL3.
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
---
target-arm/cpu.h | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 101d139..c000716 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -100,7 +100,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,
struct arm_boot_info;
-#define NB_MMU_MODES 2
+#define NB_MMU_MODES 4
/* We currently assume float and double are IEEE single and double
precision respectively.
@@ -753,7 +753,6 @@ static inline int arm_feature(CPUARMState *env, int
feature)
return (env-features (1ULL feature)) != 0;
}
-
Stray whitespace change.
/* Return true if exception level below EL3 is in secure state */
static inline bool arm_is_secure_below_el3(CPUARMState *env)
{
@@ -794,11 +793,12 @@ static inline bool arm_is_secure(CPUARMState *env)
/* Return true if the specified exception level is running in AArch64 state.
*/
static inline bool arm_el_is_aa64(CPUARMState *env, int el)
{
-/* We don't currently support EL2 or EL3, and this isn't valid for EL0
+/* We don't currently support EL2, and this isn't valid for EL0
* (if we're in EL0, is_a64() is what you want, and if we're not in EL0
* then the state of EL0 isn't well defined.)
*/
-assert(el == 1);
+assert(el == 1 || el == 3);
+
/* AArch64-capable CPUs always run with EL1 in AArch64 mode. This
* is a QEMU-imposed simplification which we may wish to change later.
* If we in future support EL2 and/or EL3, then the state of lower
@@ -990,9 +990,12 @@ static inline int arm_current_el(CPUARMState *env)
if ((env-uncached_cpsr 0x1f) == ARM_CPU_MODE_USR) {
return 0;
+} else if (arm_is_secure(env)) {
+/* Secure PL1 and monitor mode are mapped to PL3 */
+return 3;
This isn't correct. Secure privileged !Mon AArch32 modes are only
EL3 if EL3 is AArch32. If EL3 is AArch64 then the !Mon AArch32
modes are EL1.
}
-/* We don't currently implement the Virtualization or TrustZone
- * extensions, so PL2 and PL3 don't exist for us.
+/* We currently do not implement the Virtualization extensions, so PL2
does
+ * not exist for us.
*/
return 1;
Now that we've added the complications for handling secure mode,
we might as well also have the trivial code for Hyp too. So
that means the function looks something like:
if (env-aarch64) {
return extract32(env-pstate, 2, 2);
}
switch (env-uncached_cpsr CPSR_M) {
case ARM_CPU_MODE_USR:
return 0;
case ARM_CPU_MODE_HYP:
return 2;
case ARM_CPU_MODE_MON:
return 3;
default:
if (arm_is_secure(env) !arm_el_is_aa64(env, 3)) {
/* If EL3 is 32-bit then all secure privileged modes run in EL3 */
return 3;
}
return 1;
}
thanks
-- PMM
Re: [Qemu-devel] [PATCH v5 06/33] target-arm: A32: Emulate the SMC instruction
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
Implements SMC instruction in Aarch32 using the A32 syndrome. When executing
SMC instruction from monitor CPU mode SCR.NS bit is reset.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Merge pre_smc upstream changes and incorporated ss_advance
---
target-arm/helper.c| 11 +++
target-arm/internals.h | 7 ++-
target-arm/op_helper.c | 3 +--
target-arm/translate.c | 39 +--
4 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2381e6f..7f3f049 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4090,6 +4090,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
mask = CPSR_A | CPSR_I | CPSR_F;
offset = 4;
break;
+case EXCP_SMC:
+new_mode = ARM_CPU_MODE_MON;
+addr = 0x08;
+mask = CPSR_A | CPSR_I | CPSR_F;
+offset = 0;
+break;
default:
cpu_abort(cs, Unhandled exception 0x%x\n, cs-exception_index);
return; /* Never happens. Keep compiler happy. */
@@ -4108,6 +4114,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
*/
addr += env-cp15.vbar_el[1];
}
+
+if ((env-uncached_cpsr CPSR_M) == ARM_CPU_MODE_MON) {
+env-cp15.scr_el3 = ~SCR_NS;
+}
+
switch_mode (env, new_mode);
/* For exceptions taken to AArch32 we must clear the SS bit in both
* PSTATE and in the old-state value we save to SPSR_mode, so zero it
now.
diff --git a/target-arm/internals.h b/target-arm/internals.h
index fd69a83..43a2e7d 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -236,7 +236,12 @@ static inline uint32_t syn_aa32_svc(uint32_t imm16, bool
is_thumb)
| (is_thumb ? 0 : ARM_EL_IL);
}
-static inline uint32_t syn_aa64_bkpt(uint32_t imm16)
+static inline uint32_t syn_aa32_smc(void)
+{
+return (EC_AA32_SMC ARM_EL_EC_SHIFT) | ARM_EL_IL;
+}
+
+static inline uint32_t syn_aa64_bkpt(uint16_t imm16)
Bogus change (probably introduced by accident in a merge
conflict fixup).
{
return (EC_AA64_BKPT ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 0x);
}
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 0809d63..8ed8ee9 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -419,8 +419,7 @@ void HELPER(pre_hvc)(CPUARMState *env)
void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
{
int cur_el = arm_current_el(env);
-/* FIXME: Use real secure state. */
-bool secure = false;
+bool secure = arm_is_secure(env);;
Doubled semicolon.
bool smd = env-cp15.scr_el3 SCR_SMD;
/* On ARMv8 AArch32, SMD only applies to NS state.
* On ARMv7 SMD only applies to NS state and only if EL2 is available.
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f6404be..3f3ddfb 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7872,15 +7872,27 @@ static void disas_arm_insn(CPUARMState * env,
DisasContext *s)
case 7:
{
int imm16 = extract32(insn, 0, 4) | (extract32(insn, 8, 12)
4);
-/* SMC instruction (op1 == 3)
- and undefined instructions (op1 == 0 || op1 == 2)
- will trap */
-if (op1 != 1) {
+if (op1 == 1) {
+/* bkpt */
+ARCH(5);
+gen_exception_insn(s, 4, EXCP_BKPT,
+syn_aa32_bkpt(imm16, false));
+} else if (op1 == 3) {
+/* smi/smc */
+if (!arm_dc_feature(s, ARM_FEATURE_EL3) ||
+s-current_pl == 0) {
+goto illegal_op;
+}
+gen_set_pc_im(s, s-pc);
This should be s-pc - 4, because if the pre_smc helper throws
an UNDEF exception you want the PC to point to the SMC insn,
not the insn after. (If the SMC executes as an SMC then the
PC for the EXCP_SMC will be correct because of the 0 offset
passed to gen_exception_insn below.)
+tmp = tcg_const_i32(syn_aa32_smc());
+gen_helper_pre_smc(cpu_env, tmp);
+tcg_temp_free_i32(tmp);
+gen_ss_advance(s);
+gen_exception_insn(s, 0, EXCP_SMC, syn_aa32_smc());
+break;
+} else {
goto illegal_op;
}
That said, I have a feeling we might want to put the SMC
handling into the end-of-loop processing for A32/T32,
the same way we do SVC. Ard has a patch onlist which does
that, though it is on my todo list to try to fix because
it has its own issues...
-- PMM
Re: [Qemu-devel] [PATCH v5 07/33] target-arm: extend async excp masking
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
This patch extends arm_excp_unmasked() according to ARM ARMv7 and
ARM ARMv8 (all EL running in Aarch32) and adds comments.
AA (just do a search and replace through the whole patchset,
please.)
If EL3 is using Aarch64 IRQ/FIQ masking is ignored in
all exception levels other than EL3 if SCR.{FIQ|IRQ} is
set to 1 (routed to EL3).
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Merge with v4 patch 10
---
target-arm/cpu.h | 116
++-
1 file changed, 106 insertions(+), 10 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c000716..30f57fd 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1226,11 +1226,8 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx)
{
CPUARMState *env = cs-env_ptr;
unsigned int cur_el = arm_current_el(env);
-unsigned int target_el = arm_excp_target_el(cs, excp_idx);
-/* FIXME: Use actual secure state. */
-bool secure = false;
-/* If in EL1/0, Physical IRQ routing to EL2 only happens from NS state.
*/
-bool irq_can_hyp = !secure cur_el 2 target_el == 2;
+bool secure = arm_is_secure(env);
+
/* ARMv7-M interrupt return works by loading a magic value
* into the PC. On real hardware the load causes the
* return to occur. The qemu implementation performs the
@@ -1245,19 +1242,118 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx)
(!IS_M(env) || env-regs[15] 0xfff0);
/* Don't take exceptions if they target a lower EL. */
-if (cur_el target_el) {
+if (cur_el arm_excp_target_el(cs, excp_idx)) {
return false;
}
+/* ARM ARMv7 B1.8.6 Asynchronous exception masking (table B1-12/B1-13)
+ * ARM ARMv8 G1.11.3 Asynchronous exception masking controls
+ * (table G1-18/G1-19) */
*/ goes on its own line.
switch (excp_idx) {
case EXCP_FIQ:
-if (irq_can_hyp (env-cp15.hcr_el2 HCR_FMO)) {
-return true;
+if (arm_feature(env, ARM_FEATURE_EL3) arm_el_is_aa64(env, 3)) {
+/* If EL3 is using Aarch64 and FIQs are routed to EL3 masking is
+ * ignored in all exception levels except EL3.
+ */
+if ((env-cp15.scr_el3 SCR_FIQ) cur_el 3) {
+return true;
+}
+/* If we are in EL3 but FIQs are not routed to EL3 the exception
+ * is not taken but remains pending.
+ */
+if (!(env-cp15.scr_el3 SCR_FIQ) cur_el == 3) {
+return false;
+}
+}
This is all kind of confusing. What is the division of work
between this function and arm_phys_excp_target_el(),
conceptually? Why isn't we're in EL3 but FIQs aren't going
to EL3 handled by the cur_el arm_excp_target_el() check
above, for instance?
thanks
-- PMM
Re: [Qemu-devel] [PATCH v5 08/33] target-arm: add async excp target_el function
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
Adds a dedicated function for IRQ and FIQ exceptions to determine
target_el and mode (Aarch32) according to tables in ARM ARMv8 and
ARM ARM v7.
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Simplify target EL function including removal of mode which was unused
- Merged with patch that plugs in the use of the function
v3 - v4
- Fixed arm_phys_excp_target_el() 0/0/0 case to return excp_mode when EL2
rather than ABORT.
---
target-arm/cpu.h| 2 +
target-arm/helper.c | 103
++--
2 files changed, 85 insertions(+), 20 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 30f57fd..601f8fe 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -809,6 +809,8 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int
el)
void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx);
+inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+uint32_t cur_el, bool secure);
This is only used in helper.c which is also the place where it
is defined, so why are we making it a global function with
a prototype here rather than having it be 'static'?
/* Interface between CPU and Interrupt controller. */
void armv7m_nvic_set_pending(void *opaque, int irq);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7f3f049..a10f459 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3706,6 +3706,12 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env,
uint32_t mode)
return 0;
}
+inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+uint32_t cur_el, bool secure)
+{
+return 1;
+}
+
This version is never used, so I think it can be deleted?
unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
{
return 1;
@@ -3767,6 +3773,80 @@ void switch_mode(CPUARMState *env, int mode)
}
/*
+ * Determine the target EL for physical exceptions
What's a physical exception ?
+ */
+inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
+uint32_t cur_el, bool secure)
+{
+CPUARMState *env = cs-env_ptr;
+uint32_t target_el = 1;
+
+/* There is no SCR or HCR routing unless the respective EL3 and EL2
+ * extensions are supported. This initial setting affects whether any
+ * other conditions matter.
+ */
+bool scr_routing = arm_feature(env, ARM_FEATURE_EL3); /* IRQ, FIQ, EA */
+bool hcr_routing = arm_feature(env, ARM_FEATURE_EL2); /* IMO, FMO, AMO */
+
+/* Fast-path if EL2 and EL3 are not enabled */
+if (!scr_routing !hcr_routing) {
+return target_el;
+}
+
+switch (excp_idx) {
+case EXCP_IRQ:
+scr_routing = ((env-cp15.scr_el3 SCR_IRQ) == SCR_IRQ);
+hcr_routing = ((env-cp15.hcr_el2 HCR_IMO) == HCR_IMO);
+break;
+case EXCP_FIQ:
+scr_routing = ((env-cp15.scr_el3 SCR_FIQ) == SCR_FIQ);
+hcr_routing = ((env-cp15.hcr_el2 HCR_FMO) == HCR_FMO);
+}
+
+/* If SCR routing is enabled we always go to EL3 regardless of EL3
+ * execution state
+ */
+if (scr_routing) {
+/* IRQ|FIQ|EA == 1 */
+return 3;
+}
+
+/* If HCR.TGE is set all exceptions that would be routed to EL1 are
+ * routed to EL2 (in non-secure world).
+ */
+hcr_routing = (env-cp15.hcr_el2 HCR_TGE) == HCR_TGE;
+
+/* Determine target EL according to ARM ARMv8 tables G1-15 and G1-16 */
+if (arm_el_is_aa64(env, 3)) {
+/* EL3 in Aarch64 */
+if (!secure) {
+/* If non-secure, we may route to EL2 depending on other state.
+ * If we are coming from the secure world then we always route to
+ * EL1.
+ */
+if (hcr_routing ||
+(cur_el == 2 !(env-cp15.scr_el3 SCR_RW))) {
+/* If HCR.FMO/IMO is set or we already in EL2 and it is not
+ * configured to be AArch64 then route to EL2.
+ */
+target_el = 2;
+}
+}
+} else {
+/* EL3 in Aarch32 */
+if (secure) {
+/* If coming from secure always route to EL3 */
+target_el = 3;
+} else if (hcr_routing || cur_el == 2) {
+/* If HCR.FMO/IMO is set or we are already EL2 then route to EL2
*/
+target_el = 2;
+}
+}
+
+return target_el;
+}
+
+/*
* Determine the target EL for a given exception type.
*/
unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
@@
Re: [Qemu-devel] [PATCH v5 09/33] target-arm: add macros to access banked registers
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
If EL3 is in Aarch32 state certain cp registers are banked (secure and
non-secure instance). When reading or writing to coprocessor registers
the following macros can be used.
- A32_BANKED macros are used for choosing the banked register based on
provided
input security argument. This macro is used to choose the bank during
translation of MRC/MCR instructions that are dependent on something other
than the current secure state.
- A32_BANKED_CURRENT macros are used for choosing the banked register based on
current secure state. This is NOT to be used for choosing the bank used
during translation as it breaks monitor mode.
If EL3 is operating in Aarch64 state coprocessor registers are not
banked anymore. The macros use the non-secure instance (_ns) in this
case, which is architecturally mapped to the Aarch64 EL register.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Cleaned-up macros to try and alleviate misuse. Made A32_BANKED macros take
secure arg indicator rather than relying on USE_SECURE_REG. Incorporated
the
A32_BANKED macros into the A32_BANKED_CURRENT. CURRENT is now the only one
that automatically chooses based on current secure state.
---
target-arm/cpu.h | 36
1 file changed, 36 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 601f8fe..c58fdf5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -807,6 +807,42 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int
el)
return arm_feature(env, ARM_FEATURE_AARCH64);
}
+/* Macro for determing whether to use the secure or non-secure bank of a CP
+ * register. When EL3 is operating in Aarch32 state, the NS-bit determines
+ * whether the secure instance of a cp-register should be used.
+ */
+#define USE_SECURE_REG(_env) ( \
+arm_feature((_env), ARM_FEATURE_EL3) \
+!arm_el_is_aa64((_env), 3) \
+!((_env)-cp15.scr_el3 SCR_NS))
Better to use an inline function for this rather than a macro,
I think.
+
+/* Macros for accessing a specified CP register bank */
+#define A32_BANKED_REG_GET(_env, _regname, _secure)\
+((_secure) ? (_env)-cp15._regname##_s : (_env)-cp15._regname##_ns)
+
+#define A32_BANKED_REG_SET(_env, _regname, _secure, _val) \
+do {\
+if (_secure) { \
+(_env)-cp15._regname##_s = (_val);\
+} else {\
+(_env)-cp15._regname##_ns = (_val); \
+} \
+} while (0)
+
+/* Macros for automatically accessing a specific CP register bank depending
on
+ * the current secure state of the system. These macros are not intended for
+ * supporting instruction translation reads/writes as these are dependent
+ * solely on the SCR.NS bit and not the mode.
+ */
+#define A32_BANKED_CURRENT_REG_GET(_env, _regname)\
+A32_BANKED_REG_GET((_env), _regname,\
+ ((!arm_el_is_aa64((_env), 3) arm_is_secure(_env
+
+#define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val)
\
+A32_BANKED_REG_SET((_env), _regname,\
+ ((!arm_el_is_aa64((_env), 3)
arm_is_secure(_env))), \
+ (_val))
+
...though these all have to be macros because of the regname handling.
(Do we use !arm_el_is_aa64((env), 3) arm_is_secure(env)
often enough to make it worth a utility function? I can't
think of a good name though, so maybe not...)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v5 10/33] target-arm: add non-secure Translation Block flag
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Sergey Fedorov s.fedo...@samsung.com
This patch is based on idea found in patch at
git://github.com/jowinter/qemu-trustzone.git
f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
Johannes Winter johannes.win...@iaik.tugraz.at.
This flag prevents QEMU from executing TCG code generated for other CPU
security state. It also allows to generate different TCG code depending on
CPU secure state.
This doesn't quite seem to line up with the code:
the commit message says the flag is for the CPU's
current security state, but the code is using the
which register bank setting.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Merge changes
- Fixed issue where TB secure state flag was incorrectly being set based on
secure state rather than NS setting. This caused an issue where monitor
mode
MRC/MCR accesses were always secure rather than being based on NS bit
setting.
- Added separate 64/32 TB secure state flags
- Unconditionalized the setting of the DC ns bit
- Removed IS_NS macro and replaced with direct usage.
---
target-arm/cpu.h | 14 ++
target-arm/translate-a64.c | 1 +
target-arm/translate.c | 1 +
target-arm/translate.h | 1 +
4 files changed, 17 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c58fdf5..1700676 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1528,6 +1528,8 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
*/
#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 20
#define ARM_TBFLAG_XSCALE_CPAR_MASK (3 ARM_TBFLAG_XSCALE_CPAR_SHIFT)
+#define ARM_TBFLAG_NS_SHIFT 22
+#define ARM_TBFLAG_NS_MASK (1 ARM_TBFLAG_NS_SHIFT)
/* Bit usage when in AArch64 state */
#define ARM_TBFLAG_AA64_EL_SHIFT0
@@ -1538,6 +1540,8 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 4
#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
+#define ARM_TBFLAG_AA64_NS_SHIFT5
+#define ARM_TBFLAG_AA64_NS_MASK (1 ARM_TBFLAG_AA64_NS_SHIFT)
/* some convenience accessor macros */
#define ARM_TBFLAG_AARCH64_STATE(F) \
@@ -1572,6 +1576,10 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
(((F) ARM_TBFLAG_AA64_SS_ACTIVE_MASK)
ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
(((F) ARM_TBFLAG_AA64_PSTATE_SS_MASK)
ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
+#define ARM_TBFLAG_NS(F) \
+(((F) ARM_TBFLAG_NS_MASK) ARM_TBFLAG_NS_SHIFT)
+#define ARM_TBFLAG_AA64_NS(F) \
+(((F) ARM_TBFLAG_AA64_NS_MASK) ARM_TBFLAG_AA64_NS_SHIFT)
static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
target_ulong *cs_base, int *flags)
@@ -1605,6 +1613,9 @@ static inline void cpu_get_tb_cpu_state(CPUARMState
*env, target_ulong *pc,
*flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
}
}
+if (!(USE_SECURE_REG(env))) {
+*flags |= ARM_TBFLAG_AA64_NS_MASK;
+}
What's this for? If we're in AArch64 mode then we know that
EL3 (if it exists) must also be AArch64, and so USE_SECURE_REG
always returns false...
} else {
int privmode;
*pc = env-regs[15];
@@ -1621,6 +1632,9 @@ static inline void cpu_get_tb_cpu_state(CPUARMState
*env, target_ulong *pc,
if (privmode) {
*flags |= ARM_TBFLAG_PRIV_MASK;
}
+if (!(USE_SECURE_REG(env))) {
+*flags |= ARM_TBFLAG_NS_MASK;
+}
if (env-vfp.xregs[ARM_VFP_FPEXC] (1 30)
|| arm_el_is_aa64(env, 1)) {
*flags |= ARM_TBFLAG_VFPEN_MASK;
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index f53dc0f..dfc8c58 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -10926,6 +10926,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
#if !defined(CONFIG_USER_ONLY)
dc-user = (ARM_TBFLAG_AA64_EL(tb-flags) == 0);
#endif
+dc-ns = ARM_TBFLAG_AA64_NS(tb-flags);
dc-cpacr_fpen = ARM_TBFLAG_AA64_FPEN(tb-flags);
dc-vec_len = 0;
dc-vec_stride = 0;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3f3ddfb..5e1d677 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -10958,6 +10958,7 @@ static inline void
gen_intermediate_code_internal(ARMCPU *cpu,
#if !defined(CONFIG_USER_ONLY)
dc-user = (ARM_TBFLAG_PRIV(tb-flags) == 0);
#endif
+dc-ns = ARM_TBFLAG_NS(tb-flags);
dc-cpacr_fpen = ARM_TBFLAG_CPACR_FPEN(tb-flags);
dc-vfp_enabled = ARM_TBFLAG_VFPEN(tb-flags);
dc-vec_len =
Re: [Qemu-devel] [PATCH v5 11/33] target-arm: arrayfying fieldoffset for banking
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
Prepare ARMCPRegInfo to support specifying two fieldoffsets per
register definition. This will allow us to keep one register
definition for banked registers (different offsets for secure/
non-secure world).
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Added ARM CP register secure and non-secure bank flags
- Added setting of secure and non-secure flags furing registration
---
target-arm/cpu.h| 23 +++-
target-arm/helper.c | 60
+
2 files changed, 65 insertions(+), 18 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1700676..9681d45 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -958,10 +958,12 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 8))
#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 8))
#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
+#define ARM_CP_BANK_S (1 16)
+#define ARM_CP_BANK_NS (2 16)
I thought we were going to put these flags into a reginfo-secure
field? Mixing them into the 'type' bits seems unnecessarily
confusing to me.
/* Used only as a terminator for ARMCPRegInfo lists */
-#define ARM_CP_SENTINEL 0x
+#define ARM_CP_SENTINEL 0xff
/* Mask of only the flag bits in a type field */
-#define ARM_CP_FLAG_MASK 0x7f
+#define ARM_CP_FLAG_MASK 0x3007f
/* Valid values for ARMCPRegInfo state field, indicating which of
* the AArch32 and AArch64 execution states this register is visible in.
@@ -1096,6 +1098,7 @@ struct ARMCPRegInfo {
uint8_t opc0;
uint8_t opc1;
uint8_t opc2;
+
Stray whitespace change.
/* Execution state in which this register is visible: ARM_CP_STATE_* */
int state;
/* Register type: ARM_CP_* bits/values */
@@ -,12 +1114,22 @@ struct ARMCPRegInfo {
* fieldoffset is non-zero, the reset value of the register.
*/
uint64_t resetvalue;
-/* Offset of the field in CPUARMState for this register. This is not
- * needed if either:
+/* Offsets of the fields (secure/non-secure) in CPUARMState for this
+ * register. The array will be accessed by the ns bit which means the
+ * secure instance has to be at [0] while the non-secure instance must be
+ * at [1]. If a register is not banked .fieldoffset can be used, which
maps
+ * to the non-secure bank.
+ * This is not needed if either:
* 1. type is ARM_CP_CONST or one of the ARM_CP_SPECIALs
* 2. both readfn and writefn are specified
*/
-ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */
+union { /* offsetof(CPUARMState, field) */
+struct {
+ptrdiff_t fieldoffset_padding;
+ptrdiff_t fieldoffset;
...why is the padding field first? Given that we always write
fieldoffset when we put the banked versions into the hash table
I don't think it should matter, should it?
+};
+ptrdiff_t bank_fieldoffsets[2];
+};
/* Function for making any access checks for this register in addition to
* those specified by the 'access' permissions bits. If NULL, no extra
* checks required. The access check is performed at runtime, not at
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a10f459..ab38b68 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3296,22 +3296,56 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const
ARMCPRegInfo *r,
uint32_t *key = g_new(uint32_t, 1);
ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo));
int is64 = (r-type ARM_CP_64BIT) ? 1 : 0;
-if (r-state == ARM_CP_STATE_BOTH state == ARM_CP_STATE_AA32) {
-/* The AArch32 view of a shared register sees the lower 32 bits
- * of a 64 bit backing field. It is not migratable as the AArch64
- * view handles that. AArch64 also handles reset.
- * We assume it is a cp15 register if the .cp field is left unset.
- */
-if (r2-cp == 0) {
-r2-cp = 15;
+
+if (state == ARM_CP_STATE_AA32) {
+/* Clear the secure state flags and set based on incoming nsbit */
+r2-type = ~(ARM_CP_BANK_S | ARM_CP_BANK_NS);
+r2-type |= ARM_CP_BANK_S nsbit;
+
+if (r-bank_fieldoffsets[0] r-bank_fieldoffsets[1]) {
+/* Register is banked (using both entries in array).
+ * Overwriting fieldoffset as the array was only used to define
+ * banked registers but later only fieldoffset is used.
+ */
+r2-fieldoffset = r-bank_fieldoffsets[nsbit];
+
+/* If V8 is enabled then we don't need to migrate or reset the
+ * AArch32 version of the banked registers as this will be
handled
+
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
On Mon, Oct 06, 2014 at 04:51:35PM +0200, Greg Kurz wrote:
On Wed, 17 Sep 2014 20:39:25 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Sep 17, 2014 at 07:21:09PM +0200, Greg Kurz wrote:
On Sun, 14 Sep 2014 21:30:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:
Current support for bus master (clearing OK bit)
together with the need to support guests which do not
enable PCI bus mastering, leads to extra state in
VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust
in case of cross-version migration for the case when
guests use the device before setting DRIVER_OK.
Rip out VIRTIO_PCI_FLAG_BUS_MASTER_BUG and implement a simpler
work-around: treat clearing of PCI_COMMAND as a virtio reset. Old
guests never touch this bit so they will work.
As reset clears device status, DRIVER and MASTER bits are
now in sync, so we can fix up cross-version migration simply
by synchronising them, without need to detect a buggy guest
explicitly.
Drop tracking VIRTIO_PCI_FLAG_BUS_MASTER_BUG completely.
As reset makes the device quiescent, in the future we'll be able to drop
checking OK bit in a bunch of places.
Cc: Jason Wang jasow...@redhat.com
Cc: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Hi Michael,
I am not quite sure how to test this patch with my pseries based setup...
Migrating from qemu-2.1 to qemu-master ?
Cheers.
--
Greg
Exactly. And back! Pls don't forget to specify the 2.1 machine type.
Thanks!
Michael,
Nikunj and I had started to investigate the pseries breakage: the QEMU
originated reset brought by this patch clears the vq and breaks SLOF.
This isn't a surprise since reset should always come from the driver,
not the device.
Since commit 45363e46aeebfc99753389649eac7c7fc22bfe52 has reverted this
patch, QEMU works again for pseries and virtio. :)
So back to the initial issue, I've tried to migrate a pseries-2.1 guest
running
rhel65, from QEMU v2.1.2 to QEMU master, back and forth, several times and it
always succeeded... what symptom this patch was expected to fix ?
Cheers.
--
Greg
hw/virtio/virtio-pci.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a827cd4..f560814 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
-/* Flags track per-device state like workarounds for quirks in older
guests. */
-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 0)
-
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
VirtIOPCIProxy *dev);
@@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque,
uint32_t addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
}
-
-/* Linux before 2.6.34 sets the device as OK without enabling
- the PCI device bus master bit. In this case we need to
disable
- some safety checks. */
-if ((val VIRTIO_CONFIG_S_DRIVER_OK)
-!(proxy-pci_dev.config[PCI_COMMAND]
PCI_COMMAND_MASTER)) {
-proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
-}
break;
case VIRTIO_MSI_CONFIG_VECTOR:
msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
@@ -480,13 +469,18 @@ static void virtio_write_config(PCIDevice
*pci_dev, uint32_t address,
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev,
pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+uint8_t cmd = proxy-pci_dev.config[PCI_COMMAND];
+
pci_default_write_config(pci_dev, address, val, len);
if (range_covers_byte(address, len, PCI_COMMAND)
!(pci_dev-config[PCI_COMMAND] PCI_COMMAND_MASTER)
-!(proxy-flags VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
+(cmd PCI_COMMAND_MASTER)) {
+/* Bus driver disables bus mastering - make it act
+ * as a kind of reset to render the device quiescent. */
virtio_pci_stop_ioeventfd(proxy);
-virtio_set_status(vdev, vdev-status
~VIRTIO_CONFIG_S_DRIVER_OK);
+virtio_reset(vdev);
+msix_unuse_all_vectors(proxy-pci_dev);
}
}
@@ -895,11 +889,19 @@ static void virtio_pci_vmstate_change(DeviceState
*d, bool running)
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
if (running) {
-/* Try to find out if
Re: [Qemu-devel] [PATCH v5 12/33] target-arm: insert Aarch32 cpregs twice into hashtable
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: From: Fabian Aggeler aggel...@ethz.ch Prepare for cp register banking by inserting every cp register twice, once for secure world and once for non-secure world. Signed-off-by: Fabian Aggeler aggel...@ethz.ch Signed-off-by: Greg Bellows greg.bell...@linaro.org -- v4 - v5 - Added use of ARM CP secure/non-secure bank flags during register processing in define_one_arm_cp_reg_with_opaque(). We now only register the specified bank if only one flag is specified, otherwise we register both a secure and non-secure instance. --- target-arm/cpu.h | 14 +++--- target-arm/helper.c| 30 ++ target-arm/translate.c | 14 +- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 9681d45..220571c 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -864,6 +864,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); * Crn, Crm, opc1, opc2 fields * 32 or 64 bit register (ie is it accessed via MRC/MCR *or via MRRC/MCRR?) + * non-secure/secure bank (Aarch32 only) ...so if this is an AArch64 register is the bit in the hash table key set or clear? * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field. * (In this case crn and opc2 should be zero.) * For AArch64, there is no 32/64 bit size distinction; @@ -881,9 +882,16 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); #define CP_REG_AA64_SHIFT 28 #define CP_REG_AA64_MASK (1 CP_REG_AA64_SHIFT) -#define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2) \ -(((cp) 16) | ((is64) 15) | ((crn) 11) |\ - ((crm) 7) | ((opc1) 3) | (opc2)) +/* To enable banking of coprocessor registers depending on ns-bit we + * add a bit to distinguish between secure and non-secure cpregs in the + * hashtable. + */ +#define CP_REG_NS_SHIFT 27 +#define CP_REG_NS_MASK(nsbit) (nsbit CP_REG_NS_SHIFT) Bit 27 is already used, as part of the COPROC field. There's a reason the AA64 bit is 28... + +#define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2, ns) \ +(CP_REG_NS_MASK(ns) | ((cp) 16) | ((is64) 15) | \ + ((crn) 11) | ((crm) 7) | ((opc1) 3) | (opc2)) Doesn't this break KVM's accessing of the hashtable? You probably need to make kvm_to_cpreg_id OR in the NS bit as appropriate, and cpreg_to_kvm_id mask it out, since if we're using KVM then we're always non-secure. thanks -- PMM
Re: [Qemu-devel] [PATCH v5 32/33] target-arm: add GDB scr register
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: Added the ability to print the scr register like can be done with the cpsr. Signed-off-by: Greg Bellows greg.bell...@linaro.org Not sure you can just arbitrarily add new core registers if gdb isn't expecting them, and in any case if we want to do this we should probably do it via some more generic mechanism than manually adding registers one at a time. I recommend you just drop this patch for now. thanks -- PMM
Re: [Qemu-devel] [PATCH v5 33/33] target-arm: add cpu feature EL3 to CPUs with Security Extensions
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: From: Fabian Aggeler aggel...@ethz.ch Set ARM_FEATURE_EL3 feature for CPUs that implement Security Extensions. Signed-off-by: Fabian Aggeler aggel...@ethz.ch Signed-off-by: Greg Bellows greg.bell...@linaro.org (as we've discussed, but just as a note for the wider audience:) This is the patch we can't commit til we've thought through possible back-compatibility breakage a bit more carefully. thanks -- PMM
Re: [Qemu-devel] [PATCH v5 00/33] target-arm: add Security Extensions for CPUs
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote: Version 5 of the ARM processor security extension (TrustZone) support. This patchset includes changes to support the processor security extensions on ARMv7 aarch32 with hooks for later enabling v8 aarch64/32. Thanks. I've reviewed the first dozen or so patches, and I think at that point the later patches start to need enough changes based on review comments on the first ones that it's not worth reviewing them at the moment. If you could respin and send out a v6 with the comments so far addressed that would be great. We're into soft freeze at this point, so my intention with these patches is to commit as many of the preliminary ones as we can get definitely reviewed over the next week or so, since they don't actually change behaviour for the existing CPUs. After that the freeze starts to get solid enough that anything remaining we'll just have to do review on so it's ready to commit when we finish the 2.2 release in early December, I'm afraid. -- PMM
Re: [Qemu-devel] [PATCH 10/17] mm: rmap preparation for remap_anon_pages
Hello, On Mon, Oct 06, 2014 at 09:55:41AM +0100, Dr. David Alan Gilbert wrote: * Linus Torvalds (torva...@linux-foundation.org) wrote: On Fri, Oct 3, 2014 at 10:08 AM, Andrea Arcangeli aarca...@redhat.com wrote: Overall this looks a fairly small change to the rmap code, notably less intrusive than the nonlinear vmas created by remap_file_pages. Considering that remap_file_pages() was an unmitigated disaster, and -mm has a patch to remove it entirely, I'm not at all convinced this is a good argument. We thought remap_file_pages() was a good idea, and it really really really wasn't. Almost nobody used it, why would the anonymous page case be any different? I've posted code that uses this interface to qemu-devel and it works nicely; so chalk up at least one user. For the postcopy case I'm using it for, we need to place a page, atomically some thread might try and access it, and must either 1) get caught by userfault etc or 2) must succeed in it's access and we'll have that happening somewhere between thousands and millions of times to pages in no particular order, so we need to avoid creating millions of mappings. Yes, that's our current use case. Of course if somebody has better ideas on how to resolve an anonymous userfault they're welcome. How to resolve an userfault is orthogonal on how to detect it and to notify userland about it and to be notified when the userfault has been resolved. The latter is what the userfault and userfaultfd do. The former is what remap_anon_pages is used for but we could use something else too if there are better ways. mremap would clearly work too, but it would be less strict (it could lead to silent data corruption if there are bugs in the userland code), it would be slower and it would eventually a hit a -ENOMEM failure because there would be too many vmas. I could in theory drop remap_anon_pages from this patchset, but without an optimal way to resolve an userfault, the rest isn't so useful. We're currently discussing on what would be the best way to resolve a MAP_SHARED userfault on tmpfs in fact (that's not sorted yet), but so far, it seems remap_anon_pages fits the bill for anonymous memory. remap_anon_pages is not as problematic to maintain as remap_file_pages for the reason explained in the commit header, but there are other reasons: it doesn't require special pte_file and it changes nothing of how anonymous page faults works. All it requires is a loop to catch a changed page-index (previously page-index couldn't change, not it can, that's the only thing it changes). remap_file_pages complexity derives from not being allowed to change page-index during a move because the page_mapping may be bigger than 1, while that is precisely what remap_anon_pages does. As long as this rmap preparation is the only constraints that remap_anon_pages introduces in terms of rmap, it looks a nice not-too-intrusive solution to resolve anonymous userfaults efficiently. Introducing remap_anon_pages in fact doesn't reduce the simplification derived from the removal of remap_file_pages. As opposed removing remap_anon_pages later would only have the benefit of removing this very patch 10/17 and no other benefit. In short remap_anon_pages does this (heavily simplified): pte = *src_pte; *src_pte = 0; pte_page(pte)-index = adjusted according to src_vma/dst_vma-vm_pgoff *dst_pte = pte; It guarantees not to modify the vmas and in turn it doesn't require to take the mmap_sem for writing. To use remap_anon_pages, each thread has to create its own temporary vma with MADV_DONTFORK set on it (not formally required by the syscall strict checks, but then the application must never fork if MADV_DONTFORK isn't set or remap_anon_pages could return -EBUSY: there's no risk of silent data corruption even if the thread forks without setting MADV_DONTFORK) as source region where receive data through the network. Then after the data is fully received rmap_anon_pages moves the page from the temporary vma to the address where the userfault triggered atomically (while other threads may be attempting to access the userfault address too, thanks to remap_anon_pages atomic behavior they won't risk to ever see partial data coming from the network). remap_anon_pages as side effect creates an hole in the temporary (source) vma, so the next recv() syscall receiving data from the network will fault-in a new anonymous page without requiring any further malloc/free or other kind of vma mangling. Thanks, Andrea
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
On Mon, 6 Oct 2014 19:26:21 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Oct 06, 2014 at 04:51:35PM +0200, Greg Kurz wrote:
On Wed, 17 Sep 2014 20:39:25 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Sep 17, 2014 at 07:21:09PM +0200, Greg Kurz wrote:
On Sun, 14 Sep 2014 21:30:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:
Current support for bus master (clearing OK bit)
together with the need to support guests which do not
enable PCI bus mastering, leads to extra state in
VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust
in case of cross-version migration for the case when
guests use the device before setting DRIVER_OK.
Rip out VIRTIO_PCI_FLAG_BUS_MASTER_BUG and implement a simpler
work-around: treat clearing of PCI_COMMAND as a virtio reset. Old
guests never touch this bit so they will work.
As reset clears device status, DRIVER and MASTER bits are
now in sync, so we can fix up cross-version migration simply
by synchronising them, without need to detect a buggy guest
explicitly.
Drop tracking VIRTIO_PCI_FLAG_BUS_MASTER_BUG completely.
As reset makes the device quiescent, in the future we'll be able to
drop
checking OK bit in a bunch of places.
Cc: Jason Wang jasow...@redhat.com
Cc: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Hi Michael,
I am not quite sure how to test this patch with my pseries based
setup...
Migrating from qemu-2.1 to qemu-master ?
Cheers.
--
Greg
Exactly. And back! Pls don't forget to specify the 2.1 machine type.
Thanks!
Michael,
Nikunj and I had started to investigate the pseries breakage: the QEMU
originated reset brought by this patch clears the vq and breaks SLOF.
This isn't a surprise since reset should always come from the driver,
not the device.
Since commit 45363e46aeebfc99753389649eac7c7fc22bfe52 has reverted this
patch, QEMU works again for pseries and virtio. :)
So back to the initial issue, I've tried to migrate a pseries-2.1 guest
running
rhel65, from QEMU v2.1.2 to QEMU master, back and forth, several times and
it
always succeeded... what symptom this patch was expected to fix ?
Cheers.
--
Greg
hw/virtio/virtio-pci.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a827cd4..f560814 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
-/* Flags track per-device state like workarounds for quirks in older
guests. */
-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 0)
-
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
VirtIOPCIProxy *dev);
@@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque,
uint32_t addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
}
-
-/* Linux before 2.6.34 sets the device as OK without enabling
- the PCI device bus master bit. In this case we need to
disable
- some safety checks. */
-if ((val VIRTIO_CONFIG_S_DRIVER_OK)
-!(proxy-pci_dev.config[PCI_COMMAND]
PCI_COMMAND_MASTER)) {
-proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
-}
break;
case VIRTIO_MSI_CONFIG_VECTOR:
msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
@@ -480,13 +469,18 @@ static void virtio_write_config(PCIDevice
*pci_dev, uint32_t address,
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev,
pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+uint8_t cmd = proxy-pci_dev.config[PCI_COMMAND];
+
pci_default_write_config(pci_dev, address, val, len);
if (range_covers_byte(address, len, PCI_COMMAND)
!(pci_dev-config[PCI_COMMAND] PCI_COMMAND_MASTER)
-!(proxy-flags VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
+(cmd PCI_COMMAND_MASTER)) {
+/* Bus driver disables bus mastering - make it act
+ * as a kind of reset to render the device quiescent. */
virtio_pci_stop_ioeventfd(proxy);
-virtio_set_status(vdev, vdev-status
~VIRTIO_CONFIG_S_DRIVER_OK);
+virtio_reset(vdev);
+msix_unuse_all_vectors(proxy-pci_dev);
}
}
@@
Re: [Qemu-devel] [PULL 0/5] linux-user patches for 2.2
On 6 October 2014 15:59, Peter Maydell peter.mayd...@linaro.org wrote: Hi. I'm afraid this doesn't compile on my ARM box: /root/qemu/linux-user/syscall.c: In function ‘do_syscall’: /root/qemu/linux-user/syscall.c:9695:9: error: implicit declaration of function ‘timerfd_create’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9695:9: error: nested extern declaration of ‘timerfd_create’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9705:13: error: implicit declaration of function ‘timerfd_gettime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9705:13: error: nested extern declaration of ‘timerfd_gettime’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9728:13: error: implicit declaration of function ‘timerfd_settime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9728:13: error: nested extern declaration of ‘timerfd_settime’ [-Werror=nested-externs] cc1: all warnings being treated as errors Specifically, this is because of the patch which adds #ifdef CONFIG_TIMERFD ... #endif -- it is doing so earlier in the file than the include of qemu-common.h which pulls in the file defining the CONFIG_* macros, so sys/timerfd.h is now never included. -- PMM
Re: [Qemu-devel] Question on qemu threads
Thank You, Brian and Stefan! -a On Thu, Oct 2, 2014 at 11:15 AM, Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Sep 30, 2014 at 01:44:48PM -0400, Al Patel wrote: In the current system, what are the extra threads? The set of thread is dynamic because worker threads are started and terminated depending on guest activity. You cannot make assumptions about threads and QEMU provides monitor commands that expose the tids. Secondly, I am currently not using libvirt and having to start qemu from command line. I still want to pin the vcpu to a pcpu and want to use taskset on a thread. Unless I know which thread is emulating the vcpu how can I pin that thread? Do you have any other thoughts on the pinning part? Use libvirt if you can. It will save you a ton of time. If you really cannot use it, you can still look at its source code to understand how it does what it does. See qemuProcessDetectVcpuPIDs() in libvirt and the QEMU query-cpus QMP command. Stefan
Re: [Qemu-devel] [PATCH] gdb: provide the name of the architecture in the target.xml
On 6 October 2014 16:08, Cornelia Huck cornelia.h...@de.ibm.com wrote: On Tue, 30 Sep 2014 17:23:47 +0200 Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com Acked-by: Cornelia Huck cornelia.h...@de.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Reviewed-by: Andreas Färber afaer...@suse.de Cc: Andrzej Zaborowski balr...@gmail.com Cc: Peter Maydell peter.mayd...@linaro.org Cc: Vassili Karpov (malc) av1...@comtv.ru CC: Edgar Iglesias edgar.igles...@gmail.com CC: Richard Henderson r...@twiddle.net --- gdbstub.c | 19 --- include/qom/cpu.h | 2 ++ target-arm/cpu64.c | 1 + target-ppc/translate_init.c | 2 ++ target-s390x/cpu.c | 1 + 5 files changed, 18 insertions(+), 7 deletions(-) I will send this with the next pile of s390x updates, unless someone on cc: has any objections. I'm still hoping for an answer about why this is setting the name for 64 bit ARM and not 32 bit ARM, and whether there are any cases which need to actually be able to set the architecture name in a more complicated name than simply a string. I raised those in the last lot of review and there doesn't seem to have been any answer. thanks -- PMM
Re: [Qemu-devel] [PATCH] gdb: provide the name of the architecture in the target.xml
On 30 September 2014 16:23, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. gdb seems to support more than one powerpc architecture name. Do we need to report powerpc:e500 for our e500 cpu models, for instance? -- PMM
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
On Mon, Oct 06, 2014 at 06:46:17PM +0200, Greg Kurz wrote:
On Mon, 6 Oct 2014 19:26:21 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Oct 06, 2014 at 04:51:35PM +0200, Greg Kurz wrote:
On Wed, 17 Sep 2014 20:39:25 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Sep 17, 2014 at 07:21:09PM +0200, Greg Kurz wrote:
On Sun, 14 Sep 2014 21:30:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:
Current support for bus master (clearing OK bit)
together with the need to support guests which do not
enable PCI bus mastering, leads to extra state in
VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust
in case of cross-version migration for the case when
guests use the device before setting DRIVER_OK.
Rip out VIRTIO_PCI_FLAG_BUS_MASTER_BUG and implement a simpler
work-around: treat clearing of PCI_COMMAND as a virtio reset. Old
guests never touch this bit so they will work.
As reset clears device status, DRIVER and MASTER bits are
now in sync, so we can fix up cross-version migration simply
by synchronising them, without need to detect a buggy guest
explicitly.
Drop tracking VIRTIO_PCI_FLAG_BUS_MASTER_BUG completely.
As reset makes the device quiescent, in the future we'll be able to
drop
checking OK bit in a bunch of places.
Cc: Jason Wang jasow...@redhat.com
Cc: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
Hi Michael,
I am not quite sure how to test this patch with my pseries based
setup...
Migrating from qemu-2.1 to qemu-master ?
Cheers.
--
Greg
Exactly. And back! Pls don't forget to specify the 2.1 machine type.
Thanks!
Michael,
Nikunj and I had started to investigate the pseries breakage: the QEMU
originated reset brought by this patch clears the vq and breaks SLOF.
This isn't a surprise since reset should always come from the driver,
not the device.
Since commit 45363e46aeebfc99753389649eac7c7fc22bfe52 has reverted this
patch, QEMU works again for pseries and virtio. :)
So back to the initial issue, I've tried to migrate a pseries-2.1 guest
running
rhel65, from QEMU v2.1.2 to QEMU master, back and forth, several times
and it
always succeeded... what symptom this patch was expected to fix ?
Cheers.
--
Greg
hw/virtio/virtio-pci.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a827cd4..f560814 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
-/* Flags track per-device state like workarounds for quirks in
older guests. */
-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 0)
-
static void virtio_pci_bus_new(VirtioBusState *bus, size_t
bus_size,
VirtIOPCIProxy *dev);
@@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque,
uint32_t addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
}
-
-/* Linux before 2.6.34 sets the device as OK without
enabling
- the PCI device bus master bit. In this case we need to
disable
- some safety checks. */
-if ((val VIRTIO_CONFIG_S_DRIVER_OK)
-!(proxy-pci_dev.config[PCI_COMMAND]
PCI_COMMAND_MASTER)) {
-proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
-}
break;
case VIRTIO_MSI_CONFIG_VECTOR:
msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
@@ -480,13 +469,18 @@ static void virtio_write_config(PCIDevice
*pci_dev, uint32_t address,
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev,
pci_dev);
VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+uint8_t cmd = proxy-pci_dev.config[PCI_COMMAND];
+
pci_default_write_config(pci_dev, address, val, len);
if (range_covers_byte(address, len, PCI_COMMAND)
!(pci_dev-config[PCI_COMMAND] PCI_COMMAND_MASTER)
-!(proxy-flags VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
+(cmd PCI_COMMAND_MASTER)) {
+/* Bus driver disables bus mastering - make it act
+ * as a kind of reset to render the device quiescent. */
virtio_pci_stop_ioeventfd(proxy);
-
Re: [Qemu-devel] [PATCH 08/17] mm: madvise MADV_USERFAULT
Hi, On Sat, Oct 04, 2014 at 08:13:36AM +0900, Mike Hommey wrote: On Fri, Oct 03, 2014 at 07:07:58PM +0200, Andrea Arcangeli wrote: MADV_USERFAULT is a new madvise flag that will set VM_USERFAULT in the vma flags. Whenever VM_USERFAULT is set in an anonymous vma, if userland touches a still unmapped virtual address, a sigbus signal is sent instead of allocating a new page. The sigbus signal handler will then resolve the page fault in userland by calling the remap_anon_pages syscall. What does unmapped virtual address mean in this context? To clarify this I added this in a second sentence in the commit header: still unmapped virtual address of the previous sentence in this context means that the pte/trans_huge_pmd is null. It means it's an hole inside the anonymous vma (the kind of hole that doesn't account for RSS but only virtual size of the process). It is the same state all anonymous virtual memory is, right after mmap. The same state that if you read from it, will map a zeropage into the faulting virtual address. If the page is swapped out, it will not trigger userfaults. If something isn't clear let me know. Thanks, Andrea
Re: [Qemu-devel] [PATCH v5 02/33] target-arm: add arm_is_secure() function
On 06.10.2014 07:56, Peter Maydell wrote:
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Fabian Aggeler aggel...@ethz.ch
arm_is_secure() function allows to determine CPU security state
if the CPU implements Security Extensions/EL3.
arm_is_secure_below_el3() returns true if CPU is in secure state
below EL3.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
---
target-arm/cpu.h | 38 ++
1 file changed, 38 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 81fffd2..10afef0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -753,6 +753,44 @@ static inline int arm_feature(CPUARMState *env, int
feature)
return (env-features (1ULL feature)) != 0;
}
+
+/* Return true if exception level below EL3 is in secure state */
+static inline bool arm_is_secure_below_el3(CPUARMState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+return !(env-cp15.scr_el3 SCR_NS);
+} else if (arm_feature(env, ARM_FEATURE_EL2)) {
+return false;
+} else {
+/* IMPDEF: QEMU defaults to non-secure */
+return false;
I would be happy to fold both these identical 'return false'
cases together and have a comment that it's only IMPDEF
if EL2 isn't implemented.
+}
+#else
+return false;
+#endif
+}
+
+/* Return true if the processor is in secure state */
+static inline bool arm_is_secure(CPUARMState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+if (env-aarch64 extract32(env-pstate, 2, 2) == 3) {
+/* CPU currently in Aarch64 state and EL3 */
Nit: AArch64 with two capital 'A's (here and elsewhere).
+return true;
+} else if (!env-aarch64
+(env-uncached_cpsr CPSR_M) == ARM_CPU_MODE_MON) {
+/* CPU currently in Aarch32 state and monitor mode */
+return true;
+}
+}
+return arm_is_secure_below_el3(env);
+#else
+return false;
+#endif
+}
I checked your git tree and we don't actually use
arm_is_secure_below_el3() anywhere except in
arm_is_secure(), do we? That suggests to me we should
just fold the two functions together.
Can these functions live in internals.h rather than cpu.h?
(The difference is that internals.h is restricted to only
target-arm/ code whereas cpu.h is auto-included for a much
wider set of files.)
Probably arm_is_secure() would be used by ARM GIC emulation until there
is no better way to determine memory transaction NS tag.
thanks
-- PMM
[Qemu-devel] [PATCH 0/2] qemu-char: Clean up socket reporting
The first patch fixes some reporting issues for reconnect sockets that fail to connect. It prevents a constant stream of error messages, and reports the error immediately instead of delaying it. This required a small restructure, but nothing big. The second adds an error to non-blocking connect reporting. The error was available, it just wasn't being passed. Hopefully this makes the error easier to diagnose. -corey
[Qemu-devel] [PATCH 1/2] qemu-char: Fix reconnect socket error reporting
From: Corey Minyard cminy...@mvista.com
If reconnect was set, errors wouldn't always be reported.
Fix that and also only report a connect error once until a
connection has been made.
The primary purpose of this is to tell the user that a
connection failed so they can know they need to figure out
what went wrong. So we don't want to spew too much
out here, just enough so they know.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 47 ---
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 62af0ef..fb895c7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2509,6 +2509,7 @@ typedef struct {
guint reconnect_timer;
int64_t reconnect_time;
+bool connect_err_reported;
} TCPCharDriver;
static gboolean socket_reconnect_timeout(gpointer opaque);
@@ -2521,6 +2522,17 @@ static void
qemu_chr_socket_restart_timer(CharDriverState *chr)
socket_reconnect_timeout, chr);
}
+static void check_report_connect_error(CharDriverState *chr, const char *str)
+{
+TCPCharDriver *s = chr-opaque;
+
+if (!s-connect_err_reported) {
+error_report(%s char device %s\n, str, chr-label);
+s-connect_err_reported = 1;
+}
+qemu_chr_socket_restart_timer(chr);
+}
+
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void
*opaque);
#ifndef _WIN32
@@ -3045,12 +3057,14 @@ static void
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd)
static void qemu_chr_socket_connected(int fd, void *opaque)
{
CharDriverState *chr = opaque;
+TCPCharDriver *s = chr-opaque;
if (fd 0) {
-qemu_chr_socket_restart_timer(chr);
+check_report_connect_error(chr, Unable to connect to);
return;
}
+s-connect_err_reported = 0;
qemu_chr_finish_socket_connection(chr, fd);
}
@@ -4066,11 +4080,19 @@ static CharDriverState
*qmp_chardev_open_parallel(ChardevHostdev *parallel,
#endif /* WIN32 */
+static void socket_try_connect(CharDriverState *chr)
+{
+Error *err = NULL;
+
+if (!qemu_chr_open_socket_fd(chr, err)) {
+check_report_connect_error(chr, Unable to start connection to);
+}
+}
+
static gboolean socket_reconnect_timeout(gpointer opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
-Error *err;
s-reconnect_timer = 0;
@@ -4078,10 +4100,7 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
return false;
}
-if (!qemu_chr_open_socket_fd(chr, err)) {
-error_report(Unable to connect to char device %s\n, chr-label);
-qemu_chr_socket_restart_timer(chr);
-}
+socket_try_connect(chr);
return false;
}
@@ -4133,15 +4152,13 @@ static CharDriverState
*qmp_chardev_open_socket(ChardevSocket *sock,
s-reconnect_time = reconnect;
}
-if (!qemu_chr_open_socket_fd(chr, errp)) {
-if (s-reconnect_time) {
-qemu_chr_socket_restart_timer(chr);
-} else {
-g_free(s);
-g_free(chr-filename);
-g_free(chr);
-return NULL;
-}
+if (s-reconnect_time) {
+socket_try_connect(chr);
+} else if (!qemu_chr_open_socket_fd(chr, errp)) {
+g_free(s);
+g_free(chr-filename);
+g_free(chr);
+return NULL;
}
if (is_listen is_waitconnect) {
--
1.8.3.1
[Qemu-devel] [PATCH 2/2] qemu-sockets: Add error to non-blocking connect handler
From: Corey Minyard cminy...@mvista.com
An error value here would be quite handy and more consistent
with the rest of the code.
Corey Minyard cminy...@mvista.com
---
include/qemu/sockets.h | 2 +-
migration-tcp.c| 4 ++--
migration-unix.c | 4 ++--
qemu-char.c| 12 +++-
util/qemu-sockets.c| 19 ++-
5 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index fdbb196..f47dae6 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -47,7 +47,7 @@ int recv_all(int fd, void *buf, int len1, bool single_read);
/* callback function for nonblocking connect
* valid fd on success, negative error code on failure
*/
-typedef void NonBlockingConnectHandler(int fd, void *opaque);
+typedef void NonBlockingConnectHandler(int fd, Error *errp, void *opaque);
InetSocketAddress *inet_parse(const char *str, Error **errp);
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
diff --git a/migration-tcp.c b/migration-tcp.c
index 2e34517..91c9cf3 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -33,12 +33,12 @@
do { } while (0)
#endif
-static void tcp_wait_for_connect(int fd, void *opaque)
+static void tcp_wait_for_connect(int fd, Error *err, void *opaque)
{
MigrationState *s = opaque;
if (fd 0) {
-DPRINTF(migrate connect error\n);
+DPRINTF(migrate connect error: %s\n, error_get_pretty(err));
s-file = NULL;
migrate_fd_error(s);
} else {
diff --git a/migration-unix.c b/migration-unix.c
index 0a5f8a1..1cdadfb 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -33,12 +33,12 @@
do { } while (0)
#endif
-static void unix_wait_for_connect(int fd, void *opaque)
+static void unix_wait_for_connect(int fd, Error *err, void *opaque)
{
MigrationState *s = opaque;
if (fd 0) {
-DPRINTF(migrate connect error\n);
+DPRINTF(migrate connect error: %s\n, error_get_pretty(err));
s-file = NULL;
migrate_fd_error(s);
} else {
diff --git a/qemu-char.c b/qemu-char.c
index fb895c7..22a2eb7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2522,12 +2522,14 @@ static void
qemu_chr_socket_restart_timer(CharDriverState *chr)
socket_reconnect_timeout, chr);
}
-static void check_report_connect_error(CharDriverState *chr, const char *str)
+static void check_report_connect_error(CharDriverState *chr, const char *str,
+ Error *err)
{
TCPCharDriver *s = chr-opaque;
if (!s-connect_err_reported) {
-error_report(%s char device %s\n, str, chr-label);
+error_report(%s char device %s: %s\n, str, chr-label,
+ error_get_pretty(err));
s-connect_err_reported = 1;
}
qemu_chr_socket_restart_timer(chr);
@@ -3054,13 +3056,13 @@ static void
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd)
}
}
-static void qemu_chr_socket_connected(int fd, void *opaque)
+static void qemu_chr_socket_connected(int fd, Error *err, void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
if (fd 0) {
-check_report_connect_error(chr, Unable to connect to);
+check_report_connect_error(chr, Unable to connect to, err);
return;
}
@@ -4085,7 +4087,7 @@ static void socket_try_connect(CharDriverState *chr)
Error *err = NULL;
if (!qemu_chr_open_socket_fd(chr, err)) {
-check_report_connect_error(chr, Unable to start connection to);
+check_report_connect_error(chr, Unable to start connection to, err);
}
}
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 1eef590..e6a9644 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -234,6 +234,7 @@ static void wait_for_connect(void *opaque)
int val = 0, rc = 0;
socklen_t valsize = sizeof(val);
bool in_progress;
+Error *err = NULL;
qemu_set_fd_handler2(s-fd, NULL, NULL, NULL, NULL);
@@ -248,6 +249,7 @@ static void wait_for_connect(void *opaque)
/* connect error */
if (rc 0) {
+error_setg_errno(err, errno, Error connecting to socket);
closesocket(s-fd);
s-fd = rc;
}
@@ -257,9 +259,14 @@ static void wait_for_connect(void *opaque)
while (s-current_addr-ai_next != NULL s-fd 0) {
s-current_addr = s-current_addr-ai_next;
s-fd = inet_connect_addr(s-current_addr, in_progress, s, NULL);
+if (s-fd 0) {
+error_free(err);
+err = NULL;
+error_setg_errno(err, errno, Unable to start socket
connect);
+}
/* connect in progress */
if (in_progress) {
-return;
+goto out;
}
}
@@ -267,9 +274,11 @@ static void wait_for_connect(void *opaque)
}
Re: [Qemu-devel] [PATCH v5 02/33] target-arm: add arm_is_secure() function
On 6 October 2014 18:57, Sergey Fedorov serge.f...@gmail.com wrote: On 06.10.2014 07:56, Peter Maydell wrote: Can these functions live in internals.h rather than cpu.h? (The difference is that internals.h is restricted to only target-arm/ code whereas cpu.h is auto-included for a much wider set of files.) Probably arm_is_secure() would be used by ARM GIC emulation until there is no better way to determine memory transaction NS tag. We could have the GIC code temporarily include internals.h, which would be a nice big red flag that it was doing things the wrong way :-) -- PMM
Re: [Qemu-devel] [PATCH v5 10/33] target-arm: add non-secure Translation Block flag
On 06.10.2014 09:13, Peter Maydell wrote:
On 30 September 2014 22:49, Greg Bellows greg.bell...@linaro.org wrote:
From: Sergey Fedorov s.fedo...@samsung.com
This patch is based on idea found in patch at
git://github.com/jowinter/qemu-trustzone.git
f3d955c6c0ed8c46bc0eb10b634201032a651dd2 by
Johannes Winter johannes.win...@iaik.tugraz.at.
This flag prevents QEMU from executing TCG code generated for other CPU
security state. It also allows to generate different TCG code depending on
CPU secure state.
This doesn't quite seem to line up with the code:
the commit message says the flag is for the CPU's
current security state, but the code is using the
which register bank setting.
Right, the original patch used !arm_is_secure(env) as a condition for
setting this flag. But that was changed at some point without correcting
commit message.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
Signed-off-by: Greg Bellows greg.bell...@linaro.org
--
v4 - v5
- Merge changes
- Fixed issue where TB secure state flag was incorrectly being set based on
secure state rather than NS setting. This caused an issue where monitor
mode
MRC/MCR accesses were always secure rather than being based on NS bit
setting.
- Added separate 64/32 TB secure state flags
- Unconditionalized the setting of the DC ns bit
- Removed IS_NS macro and replaced with direct usage.
---
target-arm/cpu.h | 14 ++
target-arm/translate-a64.c | 1 +
target-arm/translate.c | 1 +
target-arm/translate.h | 1 +
4 files changed, 17 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c58fdf5..1700676 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1528,6 +1528,8 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
*/
#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 20
#define ARM_TBFLAG_XSCALE_CPAR_MASK (3 ARM_TBFLAG_XSCALE_CPAR_SHIFT)
+#define ARM_TBFLAG_NS_SHIFT 22
+#define ARM_TBFLAG_NS_MASK (1 ARM_TBFLAG_NS_SHIFT)
/* Bit usage when in AArch64 state */
#define ARM_TBFLAG_AA64_EL_SHIFT0
@@ -1538,6 +1540,8 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1
ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 4
#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1
ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
+#define ARM_TBFLAG_AA64_NS_SHIFT5
+#define ARM_TBFLAG_AA64_NS_MASK (1 ARM_TBFLAG_AA64_NS_SHIFT)
/* some convenience accessor macros */
#define ARM_TBFLAG_AARCH64_STATE(F) \
@@ -1572,6 +1576,10 @@ static inline bool arm_singlestep_active(CPUARMState
*env)
(((F) ARM_TBFLAG_AA64_SS_ACTIVE_MASK)
ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT)
#define ARM_TBFLAG_AA64_PSTATE_SS(F) \
(((F) ARM_TBFLAG_AA64_PSTATE_SS_MASK)
ARM_TBFLAG_AA64_PSTATE_SS_SHIFT)
+#define ARM_TBFLAG_NS(F) \
+(((F) ARM_TBFLAG_NS_MASK) ARM_TBFLAG_NS_SHIFT)
+#define ARM_TBFLAG_AA64_NS(F) \
+(((F) ARM_TBFLAG_AA64_NS_MASK) ARM_TBFLAG_AA64_NS_SHIFT)
static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
target_ulong *cs_base, int *flags)
@@ -1605,6 +1613,9 @@ static inline void cpu_get_tb_cpu_state(CPUARMState
*env, target_ulong *pc,
*flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK;
}
}
+if (!(USE_SECURE_REG(env))) {
+*flags |= ARM_TBFLAG_AA64_NS_MASK;
+}
What's this for? If we're in AArch64 mode then we know that
EL3 (if it exists) must also be AArch64, and so USE_SECURE_REG
always returns false...
} else {
int privmode;
*pc = env-regs[15];
@@ -1621,6 +1632,9 @@ static inline void cpu_get_tb_cpu_state(CPUARMState
*env, target_ulong *pc,
if (privmode) {
*flags |= ARM_TBFLAG_PRIV_MASK;
}
+if (!(USE_SECURE_REG(env))) {
+*flags |= ARM_TBFLAG_NS_MASK;
+}
if (env-vfp.xregs[ARM_VFP_FPEXC] (1 30)
|| arm_el_is_aa64(env, 1)) {
*flags |= ARM_TBFLAG_VFPEN_MASK;
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index f53dc0f..dfc8c58 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -10926,6 +10926,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
#if !defined(CONFIG_USER_ONLY)
dc-user = (ARM_TBFLAG_AA64_EL(tb-flags) == 0);
#endif
+dc-ns = ARM_TBFLAG_AA64_NS(tb-flags);
dc-cpacr_fpen = ARM_TBFLAG_AA64_FPEN(tb-flags);
dc-vec_len = 0;
dc-vec_stride = 0;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3f3ddfb..5e1d677 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -10958,6 +10958,7 @@ static inline void
gen_intermediate_code_internal(ARMCPU *cpu,
#if !defined(CONFIG_USER_ONLY)
dc-user =
Re: [Qemu-devel] [PATCH 1/2] qemu-char: Fix reconnect socket error reporting
On 10/06/2014 11:59 AM, miny...@acm.org wrote:
From: Corey Minyard cminy...@mvista.com
If reconnect was set, errors wouldn't always be reported.
Fix that and also only report a connect error once until a
connection has been made.
The primary purpose of this is to tell the user that a
connection failed so they can know they need to figure out
what went wrong. So we don't want to spew too much
out here, just enough so they know.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 47 ---
1 file changed, 32 insertions(+), 15 deletions(-)
+bool connect_err_reported;
} TCPCharDriver;
static gboolean socket_reconnect_timeout(gpointer opaque);
@@ -2521,6 +2522,17 @@ static void
qemu_chr_socket_restart_timer(CharDriverState *chr)
socket_reconnect_timeout,
chr);
}
+static void check_report_connect_error(CharDriverState *chr, const char *str)
+{
+TCPCharDriver *s = chr-opaque;
+
+if (!s-connect_err_reported) {
+error_report(%s char device %s\n, str, chr-label);
No \n at the end of an error_report() message.
+s-connect_err_reported = 1;
s/1/true/ since you typed it as bool.
+}
+qemu_chr_socket_restart_timer(chr);
+}
+
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void
*opaque);
#ifndef _WIN32
@@ -3045,12 +3057,14 @@ static void
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd)
static void qemu_chr_socket_connected(int fd, void *opaque)
{
CharDriverState *chr = opaque;
+TCPCharDriver *s = chr-opaque;
if (fd 0) {
-qemu_chr_socket_restart_timer(chr);
+check_report_connect_error(chr, Unable to connect to);
Works in English, but if there is ever translation of the message
printed in check_report_connect_error, you are probably doing
translators a disservice by splitting a sentence into two parts
separated by a number of lines of code. (Spoken by one who has never
contributed a translation, so take with a grain of salt)
return;
}
+s-connect_err_reported = 0;
s/0/false/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 38/47] Add assertion to check migration_dirty_pages
* Paolo Bonzini (pbonz...@redhat.com) wrote: Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: I've seen it go negative once during dev, it shouldn't happen. You can move it earlier, perhaps even as patch 1, since it does not have any dependency on postcopy and can go in at any time. OK, I moved it to the 2nd patch - just after the docs (Eric previously said he liked those at the start of a patch set). Dave Paolo -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 20/47] Add migration-capability boolean for postcopy-ram.
On 10/03/2014 11:47 AM, Dr. David Alan Gilbert (git) wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
include/migration/migration.h | 1 +
migration.c | 9 +
qapi-schema.json | 6 +-
3 files changed, 15 insertions(+), 1 deletion(-)
#
+# @x-postcopy-ram: Start executing on the migration target before all of RAM
has been
+# migrated, pulling the remaining pages along as needed. NOTE: If
the
+# migration fails during postcopy the VM will fail. (since 2.2)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
- 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks'] }
+ 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'x-postcopy-ram'] }
Can we wrap this to keep things in 80 columns? Also, the question was
raised on the libvirt list on whether the interface is stable enough to
name this 'postcopy-ram' from the get-go (rather than marking the
interface experimental), so that libvirt can start using it sooner.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 32/47] postcopy: ram_enable_notify to switch on userfault
* Paolo Bonzini (pbonz...@redhat.com) wrote: Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: +static int postcopy_ram_sensitise_area(const char *block_name, void *host_addr, + ram_addr_t offset, ram_addr_t length, + void *opaque) Weird name, and I'm not referring to the British -ise. :) Perhaps ram_block_enable_userfault or ram_block_enable_notify? It helps clarity to limit the use of the postcopy_ram_ prefix for static function. Yep, that's fair enough; I'll make it ram_block_enable_notify. Dave Paolo -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PULL 0/5] linux-user patches for 2.2
On Mon, Oct 06, 2014 at 05:49:14PM +0100, Peter Maydell wrote: On 6 October 2014 15:59, Peter Maydell peter.mayd...@linaro.org wrote: Hi. I'm afraid this doesn't compile on my ARM box: /root/qemu/linux-user/syscall.c: In function ‘do_syscall’: /root/qemu/linux-user/syscall.c:9695:9: error: implicit declaration of function ‘timerfd_create’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9695:9: error: nested extern declaration of ‘timerfd_create’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9705:13: error: implicit declaration of function ‘timerfd_gettime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9705:13: error: nested extern declaration of ‘timerfd_gettime’ [-Werror=nested-externs] /root/qemu/linux-user/syscall.c:9728:13: error: implicit declaration of function ‘timerfd_settime’ [-Werror=implicit-function-declaration] /root/qemu/linux-user/syscall.c:9728:13: error: nested extern declaration of ‘timerfd_settime’ [-Werror=nested-externs] cc1: all warnings being treated as errors Specifically, this is because of the patch which adds #ifdef CONFIG_TIMERFD ... #endif -- it is doing so earlier in the file than the include of qemu-common.h which pulls in the file defining the CONFIG_* macros, so sys/timerfd.h is now never included. Sorry, will fix it quickly. Riku
Re: [Qemu-devel] [PATCH v4 20/47] Add migration-capability boolean for postcopy-ram.
* Eric Blake (ebl...@redhat.com) wrote:
On 10/03/2014 11:47 AM, Dr. David Alan Gilbert (git) wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
include/migration/migration.h | 1 +
migration.c | 9 +
qapi-schema.json | 6 +-
3 files changed, 15 insertions(+), 1 deletion(-)
#
+# @x-postcopy-ram: Start executing on the migration target before all of
RAM has been
+# migrated, pulling the remaining pages along as needed. NOTE: If
the
+# migration fails during postcopy the VM will fail. (since 2.2)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
- 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks'] }
+ 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'x-postcopy-ram'] }
Can we wrap this to keep things in 80 columns?
Done.
Also, the question was
raised on the libvirt list on whether the interface is stable enough to
name this 'postcopy-ram' from the get-go (rather than marking the
interface experimental), so that libvirt can start using it sooner.
I'm still nervous about that, what I intend to do is add one
patch at the end of the series that removes the x- so that can
get discussed separately.
While I'm confident that the interface to libvirt is stable, removing the x-
declares that the whole thing is stable and I then have to maintain
migration compatibility; and it seemed sensible to let people try it for
a release; however if libvirt have no way to support QEMUs ability to have
experimental features, I guess no one is actually going to try it, which is
very disappointing.
Dave
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PULL v2 2/5] linux-user: Convert blkpg to use a special subop handler
From: Alexander Graf ag...@suse.de
The blkpg ioctl can take different payloads depending on the opcode in
its payload structure. Create a new special ioctl handler that can only
deal with partition style ones for now.
This patch fixes running parted for me.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/ioctls.h| 3 ++-
linux-user/syscall.c | 53 ++
linux-user/syscall_types.h | 2 +-
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 609b27c..e672655 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -78,7 +78,8 @@
IOCTL(BLKRAGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKSSZGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKBSZGET, IOC_R, MK_PTR(TYPE_INT))
- IOCTL(BLKPG, IOC_W, MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
+ IOCTL_SPECIAL(BLKPG, IOC_W, do_ioctl_blkpg,
+ MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
#ifdef FIBMAP
IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8fe9df7..dcb9df9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3696,6 +3696,59 @@ out:
return ret;
}
+static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t *buf_temp, int fd,
+ abi_long cmd, abi_long arg)
+{
+void *argptr;
+int target_size;
+const argtype *arg_type = ie-arg_type;
+const argtype part_arg_type[] = { MK_STRUCT(STRUCT_blkpg_partition) };
+abi_long ret;
+
+struct blkpg_ioctl_arg *host_blkpg = (void*)buf_temp;
+struct blkpg_partition host_part;
+
+/* Read and convert blkpg */
+arg_type++;
+target_size = thunk_type_size(arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(buf_temp, argptr, arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+switch (host_blkpg-op) {
+case BLKPG_ADD_PARTITION:
+case BLKPG_DEL_PARTITION:
+/* payload is struct blkpg_partition */
+break;
+default:
+/* Unknown opcode */
+ret = -TARGET_EINVAL;
+goto out;
+}
+
+/* Read and convert blkpg-data */
+arg = (abi_long)(uintptr_t)host_blkpg-data;
+target_size = thunk_type_size(part_arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(host_part, argptr, part_arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+/* Swizzle the data pointer to our local copy and call! */
+host_blkpg-data = host_part;
+ret = get_errno(ioctl(fd, ie-host_cmd, host_blkpg));
+
+out:
+return ret;
+}
+
static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
int fd, abi_long cmd, abi_long arg)
{
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 9d0c92d..1fd4ee0 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -252,4 +252,4 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* op */
TYPE_INT, /* flags */
TYPE_INT, /* datalen */
- MK_PTR(MK_STRUCT(STRUCT_blkpg_partition))) /* data */
+ TYPE_PTRVOID) /* data */
--
2.0.1
[Qemu-devel] [PULL v2 3/5] linux-user: Simplify timerid checks on g_posix_timers range
From: Alexander Graf ag...@suse.de
We check whether the passed in timer id is negative on all calls
that involve g_posix_timers.
However, these checks are bogus. First off we limit the timer_id to
16 bits which is not what Linux does. Then we check whether it's negative
which it can't be because we masked it.
We can safely remove the masking. For the negativity check we can just
treat the timerid as unsigned and only check for upper boundaries.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
linux-user/syscall.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index dcb9df9..7087a56 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9615,11 +9615,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
{
/* args: timer_t timerid, int flags, const struct itimerspec
*new_value,
* struct itimerspec * old_value */
-arg1 = 0x;
-if (arg3 == 0 || arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (arg3 == 0 || timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
target_to_host_itimerspec(hspec_new, arg3);
@@ -9635,13 +9636,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_gettime:
{
/* args: timer_t timerid, struct itimerspec *curr_value */
-arg1 = 0x;
+target_ulong timerid = arg1;
+
if (!arg2) {
return -TARGET_EFAULT;
-} else if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+} else if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec;
ret = get_errno(timer_gettime(htimer, hspec));
@@ -9657,11 +9659,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_getoverrun:
{
/* args: timer_t timerid */
-arg1 = 0x;
-if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_getoverrun(htimer));
}
break;
@@ -9672,13 +9675,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_delete:
{
/* args: timer_t timerid */
-arg1 = 0x;
-if (arg1 0 || arg1 = ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid = ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_delete(htimer));
-g_posix_timers[arg1] = 0;
+g_posix_timers[timerid] = 0;
}
break;
}
--
2.0.1
[Qemu-devel] [PULL v2 1/5] linux-user: Enable epoll_pwait syscall for ARM
From: Peter Maydell peter.mayd...@linaro.org We have support for the epoll_pwait syscall, but it wasn't enabled for ARM guests because we hadn't defined the syscall number; correct this deficiency. Reported-by: Dave Flogeras dfloger...@gmail.com Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/arm/syscall_nr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h index bef847c..7d7be7c 100644 --- a/linux-user/arm/syscall_nr.h +++ b/linux-user/arm/syscall_nr.h @@ -350,7 +350,7 @@ #define TARGET_NR_vmsplice (343) #define TARGET_NR_move_pages (344) #define TARGET_NR_getcpu (345) - /* 346 for epoll_pwait */ +#define TARGET_NR_epoll_pwait (346) #define TARGET_NR_kexec_load (347) #define TARGET_NR_utimensat(348) #define TARGET_NR_signalfd (349) -- 2.0.1
[Qemu-devel] [PULL v2 5/5] translate-all.c: memory walker initial address miscalculation
From: Mikhail Ilyin m.i...@samsung.com
The initial base address is miscalculated in walk_memory_regions().
It has to be shifted TARGET_PAGE_BITS more. Holder variables are
extended to target_ulong size otherwise they don't fit for MIPS N32
(a 32-bit ABI with a 64-bit address space) and qemu won't compile.
The issue led to incorrect debug output of memory maps and a
mis-formed coredumped file.
Signed-off-by: Mikhail Ilyin m.i...@samsung.com
Signed-off-by: Riku Voipio riku.voi...@linaro.org
---
include/exec/cpu-all.h | 4 ++--
linux-user/elfload.c | 18 +-
translate-all.c| 33 -
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9d132f..c085804 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -232,8 +232,8 @@ extern uintptr_t qemu_host_page_mask;
#if defined(CONFIG_USER_ONLY)
void page_dump(FILE *f);
-typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
- abi_ulong, unsigned long);
+typedef int (*walk_memory_regions_fn)(void *, target_ulong,
+ target_ulong, unsigned long);
int walk_memory_regions(void *, walk_memory_regions_fn);
int page_get_flags(target_ulong address);
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bea803b..1c04fcf 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2355,9 +2355,9 @@ struct elf_note_info {
};
struct vm_area_struct {
-abi_ulong vma_start; /* start vaddr of memory region */
-abi_ulong vma_end;/* end vaddr of memory region */
-abi_ulong vma_flags; /* protection etc. flags for the region */
+target_ulong vma_start; /* start vaddr of memory region */
+target_ulong vma_end;/* end vaddr of memory region */
+abi_ulong vma_flags; /* protection etc. flags for the region */
QTAILQ_ENTRY(vm_area_struct) vma_link;
};
@@ -2368,13 +2368,13 @@ struct mm_struct {
static struct mm_struct *vma_init(void);
static void vma_delete(struct mm_struct *);
-static int vma_add_mapping(struct mm_struct *, abi_ulong,
- abi_ulong, abi_ulong);
+static int vma_add_mapping(struct mm_struct *, target_ulong,
+ target_ulong, abi_ulong);
static int vma_get_mapping_count(const struct mm_struct *);
static struct vm_area_struct *vma_first(const struct mm_struct *);
static struct vm_area_struct *vma_next(struct vm_area_struct *);
static abi_ulong vma_dump_size(const struct vm_area_struct *);
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags);
static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
@@ -2466,8 +2466,8 @@ static void vma_delete(struct mm_struct *mm)
g_free(mm);
}
-static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
- abi_ulong end, abi_ulong flags)
+static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
+ target_ulong end, abi_ulong flags)
{
struct vm_area_struct *vma;
@@ -2535,7 +2535,7 @@ static abi_ulong vma_dump_size(const struct
vm_area_struct *vma)
return (vma-vma_end - vma-vma_start);
}
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags)
{
struct mm_struct *mm = (struct mm_struct *)priv;
diff --git a/translate-all.c b/translate-all.c
index 2e0265a..ba5c840 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1660,30 +1660,30 @@ void cpu_interrupt(CPUState *cpu, int mask)
struct walk_memory_regions_data {
walk_memory_regions_fn fn;
void *priv;
-uintptr_t start;
+target_ulong start;
int prot;
};
static int walk_memory_regions_end(struct walk_memory_regions_data *data,
- abi_ulong end, int new_prot)
+ target_ulong end, int new_prot)
{
-if (data-start != -1ul) {
+if (data-start != -1u) {
int rc = data-fn(data-priv, data-start, end, data-prot);
if (rc != 0) {
return rc;
}
}
-data-start = (new_prot ? end : -1ul);
+data-start = (new_prot ? end : -1u);
data-prot = new_prot;
return 0;
}
static int walk_memory_regions_1(struct walk_memory_regions_data *data,
- abi_ulong base, int level, void **lp)
+ target_ulong base, int level, void **lp)
{
-abi_ulong pa;
+target_ulong pa;
int i, rc;
if (*lp == NULL) {
@@ -1708,7 +1708,7 @@ static int walk_memory_regions_1(struct
walk_memory_regions_data *data,
void **pp = *lp;
for (i = 0; i V_L2_SIZE; ++i) {
-
[Qemu-devel] [PULL v2 0/5] linux-user patches for 2.2
From: Riku Voipio riku.voi...@linaro.org The following changes since commit 2472b6c07bb50179019589af1c22f43935ab7f5c: gdbstub: Allow target CPUs to specify watchpoint STOP_BEFORE_ACCESS flag (2014-10-06 14:25:43 +0100) are available in the git repository at: git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20141006-2 for you to fetch changes up to 1a1c4db9b298956e89caf53b09b6a7a960d55d66: translate-all.c: memory walker initial address miscalculation (2014-10-06 21:53:35 +0300) linux-user pull for 2.2 Clearest linux-user patches sent to the list since august, Apart from Mikhails patch, the rest are quite trivial. v2: check for CONFIG_TIMERFD only after it has been defined Alexander Graf (2): linux-user: Convert blkpg to use a special subop handler linux-user: Simplify timerid checks on g_posix_timers range Mikhail Ilyin (1): translate-all.c: memory walker initial address miscalculation Peter Maydell (1): linux-user: Enable epoll_pwait syscall for ARM Riku Voipio (1): linux-user: don't include timerfd if not needed include/exec/cpu-all.h | 4 ++-- linux-user/arm/syscall_nr.h | 2 +- linux-user/elfload.c| 18 +- linux-user/ioctls.h | 3 ++- linux-user/syscall.c| 87 +-- linux-user/syscall_types.h | 2 +- translate-all.c | 33 - 7 files changed, 104 insertions(+), 45 deletions(-) -- 2.0.1
[Qemu-devel] [PULL v2 4/5] linux-user: don't include timerfd if not needed
From: Riku Voipio riku.voi...@linaro.org Without this, builds on older systems fail with: qemu/linux-user/syscall.c:61:25: warning: sys/timerfd.h: No such file or directory v2: fix the usual case where CONFIG_TIMERFD is enabled.. Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 7087a56..a175cc1 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -58,7 +58,6 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include sys/shm.h #include sys/sem.h #include sys/statfs.h -#include sys/timerfd.h #include utime.h #include sys/sysinfo.h //#include sys/user.h @@ -67,6 +66,9 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include linux/wireless.h #include linux/icmp.h #include qemu-common.h +#ifdef CONFIG_TIMERFD +#include sys/timerfd.h +#endif #ifdef TARGET_GPROF #include sys/gmon.h #endif -- 2.0.1
