Re: [Qemu-devel] vm performance degradation after kvm live migration or save-restore with ETP enabled
On Tue, Jul 30, 2013 at 09:04:56AM +, Zhanghaoyu (A) wrote:
hi all,
I met similar problem to these, while performing live migration or
save-restore test on the kvm platform (qemu:1.4.0, host:suse11sp2,
guest:suse11sp2), running tele-communication software suite in
guest,
https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg00098.html
http://comments.gmane.org/gmane.comp.emulators.kvm.devel/102506
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/100592
https://bugzilla.kernel.org/show_bug.cgi?id=58771
After live migration or virsh restore [savefile], one process's CPU
utilization went up by about 30%, resulted in throughput
degradation of this process.
If EPT disabled, this problem gone.
I suspect that kvm hypervisor has business with this problem.
Based on above suspect, I want to find the two adjacent versions of
kvm-kmod which triggers this problem or not (e.g. 2.6.39, 3.0-rc1),
and analyze the differences between this two versions, or apply the
patches between this two versions by bisection method, finally find the
key patches.
Any better ideas?
Thanks,
Zhang Haoyu
I've attempted to duplicate this on a number of machines that are as
similar to yours as I am able to get my hands on, and so far have not
been able to see any performance degradation. And from what I've read in
the above links, huge pages do not seem to be part of the problem.
So, if you are in a position to bisect the kernel changes, that would
probably be the best avenue to pursue in my opinion.
Bruce
I found the first bad
commit([612819c3c6e67bac8fceaa7cc402f13b1b63f7e4] KVM: propagate fault r/w
information to gup(), allow read-only memory) which triggers this problem
by git bisecting the kvm kernel (download from
https://git.kernel.org/pub/scm/virt/kvm/kvm.git) changes.
And,
git log 612819c3c6e67bac8fceaa7cc402f13b1b63f7e4 -n 1 -p
612819c3c6e67bac8fceaa7cc402f13b1b63f7e4.log
git diff
612819c3c6e67bac8fceaa7cc402f13b1b63f7e4~1..612819c3c6e67bac8fceaa7cc4
02f13b1b63f7e4 612819c3c6e67bac8fceaa7cc402f13b1b63f7e4.diff
Then, I diffed 612819c3c6e67bac8fceaa7cc402f13b1b63f7e4.log and
612819c3c6e67bac8fceaa7cc402f13b1b63f7e4.diff,
came to a conclusion that all of the differences between
612819c3c6e67bac8fceaa7cc402f13b1b63f7e4~1 and
612819c3c6e67bac8fceaa7cc402f13b1b63f7e4
are contributed by no other than 612819c3c6e67bac8fceaa7cc402f13b1b63f7e4,
so this commit is the peace-breaker which directly or indirectly causes
the degradation.
Does the map_writable flag passed to mmu_set_spte() function have effect
on PTE's PAT flag or increase the VMEXITs induced by that guest tried to
write read-only memory?
Thanks,
Zhang Haoyu
There should be no read-only memory maps backing guest RAM.
Can you confirm map_writable = false is being passed to __direct_map? (this
should not happen, for guest RAM).
And if it is false, please capture the associated GFN.
I added below check and printk at the start of __direct_map() at the fist bad
commit version,
--- kvm-612819c3c6e67bac8fceaa7cc402f13b1b63f7e4/arch/x86/kvm/mmu.c
2013-07-26 18:44:05.0 +0800
+++ kvm-612819/arch/x86/kvm/mmu.c 2013-07-31 00:05:48.0 +0800
@@ -2223,6 +2223,9 @@ static int __direct_map(struct kvm_vcpu
int pt_write = 0;
gfn_t pseudo_gfn;
+if (!map_writable)
+printk(KERN_ERR %s: %s: gfn = %llu \n, __FILE__, __func__,
gfn);
+
for_each_shadow_entry(vcpu, (u64)gfn PAGE_SHIFT, iterator) {
if (iterator.level == level) {
unsigned pte_access = ACC_ALL;
I virsh-save the VM, and then virsh-restore it, so many GFNs were printed,
you can absolutely describe it as flooding.
The flooding you see happens during migrate to file stage because of dirty
page tracking. If you clear dmesg after virsh-save you should not see any
flooding after virsh-restore. I just checked with latest tree, I do not.
--
Gleb.
[Qemu-devel] [PATCH] monitor: fix parsing of big int
We call strtoull(3) to parse a string to int. the range we can accept
with our local variable int64_t n is (-9223372036854775808 ~
9223372036854775807), but strtoull(3) can return (0 ~
18446744073709551615UL).
So when we pass a int from HMP within the range of 9223372036854775808 ~
18446744073709551615, it's safely converted and implicitly casted to
int64_t, so n is assigned a negative value, silently, which is
incorrect. We can verify this by
(HMP) block_set_io_throttle ide0-hd0 99 0 0 0 0 0
(HMP) block_set_io_throttle ide0-hd0 999 0 0 0 0 0
bps and iops values must be 0 or greater
(HMP) block_set_io_throttle ide0-hd0 0 0 0 0 0
number too large
The first command succeeds, the second is reporting a negative value
error, the third command has correct prompt.
Fix it by calling strtoll instead, which will report ERANGE as expected.
(HMP) block_set_io_throttle ide0-hd0 99 0 0 0 0 0
(HMP) block_set_io_throttle ide0-hd0 999 0 0 0 0 0
number too large
(HMP) block_set_io_throttle ide0-hd0 0 0 0 0 0
number too large
Signed-off-by: Fam Zheng f...@redhat.com
---
monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index 5dc0aa9..7bfb469 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3286,7 +3286,7 @@ static int64_t expr_unary(Monitor *mon)
break;
default:
errno = 0;
-n = strtoull(pch, p, 0);
+n = strtoll(pch, p, 0);
if (errno == ERANGE) {
expr_error(mon, number too large);
}
--
1.8.3.4
[Qemu-devel] [PATCH] memory.c: drop kvm.h dependency
memory.c does not use any kvm specific interfaces, don't include kvm.h Signed-off-by: Michael S. Tsirkin m...@redhat.com --- memory.c | 1 - 1 file changed, 1 deletion(-) diff --git a/memory.c b/memory.c index 01846c9..6b5c420 100644 --- a/memory.c +++ b/memory.c @@ -18,7 +18,6 @@ #include exec/ioport.h #include qemu/bitops.h #include qom/object.h -#include sysemu/kvm.h #include assert.h #include exec/memory-internal.h -- MST
Re: [Qemu-devel] [PATCH 1/2] raw: add license header
On Wed, Jul 31, 2013 at 08:19:51AM +0200, Paolo Bonzini wrote: Most of the block layer is under the BSD license, thus it is reasonable to license block/raw.c the same way. CCed people should ACK by replying with a Signed-off-by line. The coded was intended to be GPLv2.
Re: [Qemu-devel] [PATCH 2/2] LICENSE: clarify
On Wed, Jul 31, 2013 at 1:19 AM, Paolo Bonzini pbonz...@redhat.com wrote: 1) The GPL says that if the Program does not specify a version number of this License, you may choose any version ever published by the Free Software Foundation. This is not true, QEMU includes parts that are v2-only. 2) Provide a default for files with no licensing information. 3) It is not just hardware emulation that is under BSD license. 4) Restrict GPLv2-only contributions to user mode emulation (due to code from Linux) and PCI passthrough (due to code from Neocleus). I am okay *discouraging* GPLv2 only contributions but if there's compelling code that cannot be relicensed, I would object strongly to rejecting it purely because it wasn't GPLv2+. Yes, that's why I included as of. We can certainly broaden the set of GPLv2-only features, but it would require discussions and another patch to LICENSE. If you believe as of July 2013 is not enough, I can send v2. 5) The rules were initially set by Fabrice but are being amended by other people (already in commit ee12e1f, LICENSE: There is no libqemu.a anymore, 2011-11-15). Do not put words in his mouth. I think it's better at this point to just put QEMU team. Fabrice is no longer associated with the project. It still mentions the trademark, and one of the last times he was contacted was exactly to enforce the BSDness of TCG, so I left it in. As the most active committer, you can certainly submit a follow-up patch to remove it; I didn't really feel qualified to do that. Paolo
Re: [Qemu-devel] [PATCH 1/2] raw: add license header
On Wed, Jul 31, 2013 at 08:19:51AM +0200, Paolo Bonzini wrote: Most of the block layer is under the BSD license, thus it is reasonable to license block/raw.c the same way. CCed people should ACK by replying with a Signed-off-by line. The coded was intended to be GPLv2. I guess some day we'll rewrite it, perhaps when libqblock is closer to being merged. Paolo
Re: [Qemu-devel] [PATCH] memory.c: drop kvm.h dependency
memory.c does not use any kvm specific interfaces, don't include kvm.h Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Paolo Bonzini pbonz...@redhat.com and adding qemu-trivial. Paolo --- memory.c | 1 - 1 file changed, 1 deletion(-) diff --git a/memory.c b/memory.c index 01846c9..6b5c420 100644 --- a/memory.c +++ b/memory.c @@ -18,7 +18,6 @@ #include exec/ioport.h #include qemu/bitops.h #include qom/object.h -#include sysemu/kvm.h #include assert.h #include exec/memory-internal.h -- MST
Re: [Qemu-devel] [PATCH 2/2] LICENSE: clarify
-1) QEMU as a whole is released under the GNU General Public License +1) QEMU as a whole is released under the GNU General Public License, +version 2. I appreciate these clarifications. For point 1, I suggest ... version 2 or (at your option) any later version. As Eric explained, I wish this was true! :) I explained this in the commit message, too: 1) The GPL says that if the Program does not specify a version number of this License, you may choose any version ever published by the Free Software Foundation. This is not true, QEMU includes parts that are v2-only. -Fabrice Bellard. +Fabrice Bellard and the QEMU team QEMU team is rather unspecific. Who is member of the QEMU team? * People with commit rights? * All maintainers listed in MAINTAINERS? * Participants of the KVM calls? * QEMU contributors working for Redhat, IBM, SUSE? * All contributors? Fabrice Bellard and all QEMU contributors would be more specific. Certainly these rules have to be understood by committers, but QEMU team is intended in the most generic sense. It just means people who care about these rules---everyone else agrees to them by providing the Signed-off-by lines in their commit messages, and this includes many from RH/IBM/SuSE. XYZ team occurs frequently in copyright notices, and I don't think the interpretation of it is going to be a problem. Paolo
Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event
On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
netclient 'name' entry in event is useful for management to know
which device is changed. n-netclient_name is not always set.
This patch changes to use nc-name. If we don't assign 'id',
qemu will set a generated name to nc-name.
Signed-off-by: Amos Kong ak...@redhat.com
Just making sure - you don't think this patch is necessary, correct?
---
hw/net/virtio-net.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c88403a..e4d9752 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -200,14 +200,9 @@ static void rxfilter_notify(NetClientState *nc)
VirtIONet *n = qemu_get_nic_opaque(nc);
if (nc-rxfilter_notify_enabled) {
-if (n-netclient_name) {
-event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
-n-netclient_name,
-
object_get_canonical_path(OBJECT(n-qdev)));
-} else {
-event_data = qobject_from_jsonf({ 'path': %s },
-
object_get_canonical_path(OBJECT(n-qdev)));
-}
+event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
+ nc-name,
+ object_get_canonical_path(OBJECT(n-qdev)));
monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
qobject_decref(event_data);
--
1.8.1.4
Re: [Qemu-devel] [PATCH] spapr-pci: rework MSI/MSIX
On Fri, Jul 12, 2013 at 05:38:24PM +1000, Alexey Kardashevskiy wrote:
On the sPAPR platform a guest allocates MSI/MSIX vectors via RTAS
hypercalls which return global IRQ numbers to a guest so it only
operates with those and never touches MSIMessage.
Therefore MSIMessage handling is completely hidden in QEMU.
Previously every sPAPR PCI host bridge implemented its own MSI window
to catch msi_notify()/msix_notify() calls from QEMU devices (virtio-pci
or vfio) and route them to the guest via qemu_pulse_irq().
MSIMessage used to be encoded as:
.addr - address within the PHB MSI window;
.data - the device index on PHB plus vector number.
The MSI MR write function translated this MSIMessage to a global IRQ
number and called qemu_pulse_irq().
However the total number of IRQs is not really big (at the moment it is
1024 IRQs starting from 4096) and even 16bit data field of MSIMessage
seems to be enough to store an IRQ number there.
This simplifies MSI handling in sPAPR PHB. Specifically, this does:
1. remove a MSI window from a PHB;
2. add a single memory region for all MSIs to sPAPREnvironment
and spapr_pci_msi_init() to initialize it;
3. encode MSIMessage as:
* .addr - a fixed address of SPAPR_PCI_MSI_WINDOW==0x400ULL;
* .data as an IRQ number.
4. change IRQ allocator to align first IRQ number in a block for MSI.
MSI uses lower bits to specify the vector number so the first IRQ has to
be aligned. MSIX does not need any special allocator though.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Acked-by: Michael S. Tsirkin m...@redhat.com
---
hw/ppc/spapr.c | 29 ++---
hw/ppc/spapr_pci.c | 61
-
include/hw/pci-host/spapr.h | 8 +++---
include/hw/ppc/spapr.h | 4 ++-
4 files changed, 60 insertions(+), 42 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 671bbd9..6b21191 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -88,6 +88,9 @@ int spapr_allocate_irq(int hint, bool lsi)
if (hint) {
irq = hint;
+if (hint = spapr-next_irq) {
+spapr-next_irq = hint + 1;
+}
/* FIXME: we should probably check for collisions somehow */
} else {
irq = spapr-next_irq++;
@@ -103,22 +106,39 @@ int spapr_allocate_irq(int hint, bool lsi)
return irq;
}
-/* Allocate block of consequtive IRQs, returns a number of the first */
-int spapr_allocate_irq_block(int num, bool lsi)
+/*
+ * Allocate block of consequtive IRQs, returns a number of the first.
+ * If msi==true, aligns the first IRQ number to num.
+ */
+int spapr_allocate_irq_block(int num, bool lsi, bool msi)
{
int first = -1;
-int i;
+int i, hint = 0;
+
+/*
+ * MSIMesage::data is used for storing VIRQ so
+ * it has to be aligned to num to support multiple
+ * MSI vectors. MSI-X is not affected by this.
+ * The hint is used for the first IRQ, the rest should
+ * be allocated continously.
+ */
+if (msi) {
+assert((num == 1) || (num == 2) || (num == 4) ||
+ (num == 8) || (num == 16) || (num == 32));
+hint = (spapr-next_irq + num - 1) ~(num - 1);
+}
for (i = 0; i num; ++i) {
int irq;
-irq = spapr_allocate_irq(0, lsi);
+irq = spapr_allocate_irq(hint, lsi);
if (!irq) {
return -1;
}
if (0 == i) {
first = irq;
+hint = 0;
}
/* If the above doesn't create a consecutive block then that's
@@ -1127,6 +1147,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
spapr_create_nvram(spapr);
/* Set up PCI */
+spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW);
spapr_pci_rtas_init();
phb = spapr_create_phb(spapr, 0);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index dfe4d04..c8ea777 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -258,11 +258,11 @@ static int spapr_msicfg_find(sPAPRPHBState *phb,
uint32_t config_addr,
* This is required for msi_notify()/msix_notify() which
* will write at the addresses via spapr_msi_write().
*/
-static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
- bool msix, unsigned req_num)
+static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr, bool msix,
+ unsigned first_irq, unsigned req_num)
{
unsigned i;
-MSIMessage msg = { .address = addr, .data = 0 };
+MSIMessage msg = { .address = addr, .data = first_irq };
if (!msix) {
msi_set_message(pdev, msg);
@@ -270,8 +270,7 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
return;
}
-for (i = 0; i req_num; ++i) {
-msg.address = addr | (i 2);
+for (i = 0; i req_num; ++i, ++msg.data) {
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Hmm, do we even need clock-using at this point? For example:
qemu_clock_enable()
{
clock-enabled = enabled;
...
if (!enabled) {
/* If another thread is within qemu_run_timers,
* wait for it to finish.
*/
qemu_event_wait(clock-callbacks_done_event);
}
}
qemu_run_timers()
{
qemu_event_reset(clock-callbacks_done_event);
if (!clock-enabled) {
goto out;
}
...
out:
qemu_event_set(eclock-callbacks_done_event);
}
In the fast path this only does two atomic operations (an OR for reset,
and XCHG for set).
There is race condition, suppose the following scenario with A/B thread
A: qemu_event_reset()
B: qemu_event_reset()
A: qemu_event_set() B is still in flight when
qemu_clock_enable() is notified
B: qemu_event_set()
I had tried to build something around futex(2) like qemu_event, but failed.
True, qemu_event basically works only when a single thread resets it. But
there is no race condition here because qemu_run_timers cannot be executed
concurrently by multiple threads (like aio_poll in your bottom half patches).
Paolo
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
--On 1 August 2013 04:57:52 -0400 Paolo Bonzini pbonz...@redhat.com wrote: True, qemu_event basically works only when a single thread resets it. But there is no race condition here because qemu_run_timers cannot be executed concurrently by multiple threads (like aio_poll in your bottom half patches). ... or, if rebasing on top of my patches, qemu_run_timers *can* be executed concurrently by mulitple threads, but in respect of any given QEMUTimerList, it will only be executed by one thread. -- Alex Bligh
Re: [Qemu-devel] [PATCH v2 3/7] block: implement reference count for BlockDriverState
On Wed, Jul 31, 2013 at 05:51:17PM +0800, Fam Zheng wrote: On Tue, 07/30 16:58, Stefan Hajnoczi wrote: On Tue, Jul 30, 2013 at 03:52:53PM +0800, Fam Zheng wrote: @@ -1518,6 +1519,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, /* dirty bitmap */ bs_dest-dirty_bitmap = bs_src-dirty_bitmap; +/* reference count */ +bs_dest-refcnt = bs_src-refcnt; + /* job */ bs_dest-in_use = bs_src-in_use; bs_dest-job= bs_src-job; Not sure this is correct, but then bdrv_swap() is hard to reason about... :) Imagine an emulated storage controller holds a reference to the BlockDriverState. When we create an external snapshot we'll bdrv_swap(old_top, new_top). We must not move new_top's refcount into old_top since the old_top object is still being referenced by the emulated storage controller. When the emulated storage controller does bdrv_unref() we'll hit the recount 0 assertion and be accessing freed memory. When we swap old_top and new_top, we want to swap all fields except for these, so we use bdrv_move_feature_fields() to move them back (bdrv_swap): tmp = *bs_new; *bs_new = *bs_old; *bs_old = tmp; /* there are some fields that should not be swapped, move them back */ bdrv_move_feature_fields(tmp, bs_old); bdrv_move_feature_fields(bs_old, bs_new); bdrv_move_feature_fields(bs_new, tmp); And I agree that refcnt is one of the fields that shouldn't be moved, so it's in bdrv_move_feature_fields(). So isn't above right? Without these lines, it *is* swapped. Yes, you are right. I should have looked at the calling function. We want to swap the refcount field back into the original struct where is belongs. Stefan
Re: [Qemu-devel] Using virtio-mmio
On 31 July 2013 23:45, Richard W.M. Jones rjo...@redhat.com wrote: ~/d/qemu/arm-softmmu/qemu-system-arm \ -m 512 -M vexpress-a9 -machine kernel_irqchip=on \ The combination of 'vexpress-a9' and kernel_irqchip=on don't make any sense -- the former implies not using KVM because KVM needs an A15 guest CPU, whereas the latter implies using KVM. In any case kernel_irqchip=on is the default when using KVM. You can just delete the kernel_irqchip option. You might want to consider -M vexpress-a15, if you want a setup that will let you use KVM (will probably need to reconfig your kernel appropriately; may need to discard earlyprintk if you compiled the kernel with the dodgy guess earlyprintk serial port address based on exact revision-and-patchlevel of CPU option.) thanks -- PMM
Re: [Qemu-devel] [PATCH 0/4] target-arm: Implement support for generic timers
Hi, On Tue, Jul 30, 2013 at 7:55 PM, Peter Maydell peter.mayd...@linaro.org wrote: This patch series implements support for the 'generic timers', which are a set of timers defined in the ARM Architecture Reference Manual and implemented by the Cortex-A15. We've got away without these up til now because Linux will generally fall back on whatever random timer is present on the devboard if it can't find the on-CPU timers, but this is less than ideal. (Among other things, the proposed mach-virt should just use the generic timers so it doesn't have to provide an sp804 timer.) Peter Maydell (4): target-arm: Allow raw_read() and raw_write() to handle 64 bit regs target-arm: Support coprocessor registers which do I/O target-arm: Implement the generic timer hw/cpu/a15mpcore: Wire generic timer outputs to GIC inputs hw/cpu/a15mpcore.c | 18 target-arm/cpu-qom.h |9 ++ target-arm/cpu.c |9 ++ target-arm/cpu.h | 24 - target-arm/helper.c| 267 ++-- target-arm/machine.c |8 +- target-arm/translate.c | 16 ++- 7 files changed, 337 insertions(+), 14 deletions(-) I tested this patch series with a 3.9 kernel for Vexpress with Cortex-A15. It works fine with the generic timer correctly detected and used. Note the 4th patch doesn't apply as is any more. Tested-by: Laurent Desnogues laurent.desnog...@gmail.com Thanks, Laurent
Re: [Qemu-devel] [PATCH v5 0/2] e1000: add interrupt mitigation support
On Wed, Jul 31, 2013 at 03:39:05PM +0200, Vincenzo Maffione wrote:
Ok, but it's unclear how do you prefer to create and empty
PC_COMPAT_1_6 in Patch 1.
If you want to keep this declaration form
[...]
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_6,
{ /* end of list */ }
},
[...]
in the two pc_*_machine_v1_6 structs, I'm forced to define
#define PC_COMPAT_1_6 { /*empty*/ }
but then I can't extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as header
(like you guys do for PC_COMPAT_1_5 and PC_COMPAT_1_4), because
otherwise PC_COMPAT_1_6 would act as a premature terminator for
PC_COMPAT_1_5 (right?).
Should I extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as a tail, or
should I avoid extending it in the Patch 1, and do the extension in
Patch 2 (when I have a non-empty PC_COMPAT_1_6)?
You are right, (GlobalProperty[]) {, {...}} is not valid syntax. In
that case I would switch PC_COMPAT_1_6 into the e1000 interrupt
mitigation patch. That way the patches are bisectable.
You can still introduce the QEMU 1.7 pc machine type as a separate
patch if you wish, but I no longer see a big win if PC_COMPAT_1_6 cannot
be isolated from the e1000 change.
Andreas: Do you agree to do everything in a single patch?
Stefan
Re: [Qemu-devel] qemu virtfs 9p not working on arm?
On 1 August 2013 00:25, Rich Felker dal...@aerifal.cx wrote: I'm not sure whether this is a kernel problem or a qemu problem (or even a user problem). The folks on OFTC #qemu suggested I email the list and relevant section maintainers so that's what I'm doing -- hope it's okay. I'm trying to boot qemu-system-arm with rootfs on 9p, and experiencing the following error: You don't say which board model you're using; I guess one of the PCI ones from the rest of the command line. | 9pnet: Installing 9P2000 support | 9pnet_virtio: probe of virtio0 failed with error -2 | VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 4 | rtc-pl031 dev:e8: setting system clock to 2013-07-31 22:56:25 UTC (1375311385) | 9pnet_virtio: no channels available | VFS: Cannot open root device root or unknown-block(0,0): error -2 | Please append a correct root= boot option; here are the available partitions: | 0b00 1048575 sr0 driver: sr | Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) That looks like ENOENT during the probe to me, but I can't find where in the kernel source it's coming from. My configuration is: -append root=root rw rootflags=rw,trans=virtio,version=9p2000.L -rootfstype=9p -fsdev local,id=root,path=$(pwd)/root,security_model=none -device virtio-9p-pci,fsdev=root,mount_tag=root and this same configuration seems to work on x86. The kernels I'm using are from the latest Aboriginal Linux images (3.10) and /proc/config.gz seems to show that all the 9p/virtfs options are enabled. Do other virtio devices work? -- PMM
[Qemu-devel] [PATCH] target-mips: fix decoding of microMIPS POOL32Axf instructions
These are not DSP instructions, thus there is no ac field.
For more details please refer to instruction encoding of
MULT, MULTU, MADD, MADDU, MSUB, MSUBU, MFHI, MFLO, MTHI, MTLO in
MIPS Architecture for Programmers Volume II-B: The microMIPS32 Instruction Set
Signed-off-by: Leon Alrae leon.al...@imgtec.com
---
target-mips/translate.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index c1d57a7..7451423 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -3,7 +3,7 @@ static void gen_pool32axf (CPUMIPSState *env,
DisasContext *ctx, int rt, int rs)
mips32_op = OPC_MSUBU;
do_mul:
check_insn(ctx, ISA_MIPS32);
-gen_muldiv(ctx, mips32_op, (ctx-opcode 14) 3, rs, rt);
+gen_muldiv(ctx, mips32_op, 0, rs, rt);
break;
default:
goto pool32axf_invalid;
@@ -11250,16 +11250,16 @@ static void gen_pool32axf (CPUMIPSState *env,
DisasContext *ctx, int rt, int rs)
case 0x35:
switch (minor 3) {
case MFHI32:
-gen_HILO(ctx, OPC_MFHI, minor 2, rs);
+gen_HILO(ctx, OPC_MFHI, 0, rs);
break;
case MFLO32:
-gen_HILO(ctx, OPC_MFLO, minor 2, rs);
+gen_HILO(ctx, OPC_MFLO, 0, rs);
break;
case MTHI32:
-gen_HILO(ctx, OPC_MTHI, minor 2, rs);
+gen_HILO(ctx, OPC_MTHI, 0, rs);
break;
case MTLO32:
-gen_HILO(ctx, OPC_MTLO, minor 2, rs);
+gen_HILO(ctx, OPC_MTLO, 0, rs);
break;
default:
goto pool32axf_invalid;
--
1.7.5.4
[Qemu-devel] [PATCH] vmdk: fix comment for vmdk_co_write_zeroes
The comment was truncated. Add the missing parts, especially explain why
we need zero_dry_run.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 3756333..e6c50b1 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1200,8 +1200,10 @@ static coroutine_fn int vmdk_co_read(BlockDriverState
*bs, int64_t sector_num,
/**
* vmdk_write:
* @zeroed: buf is ignored (data is zero), use zeroed_grain GTE feature
- * if possible, otherwise return -ENOTSUP.
- * @zero_dry_run: used for zeroed == true only, don't update L2 table, just
+ *if possible, otherwise return -ENOTSUP.
+ * @zero_dry_run: used for zeroed == true only, don't update L2 table, just try
+ *with each cluster. By dry run we can find if the zero write
+ *is possible without modifying image data.
*
* Returns: error code with 0 for success.
*/
@@ -1328,6 +1330,8 @@ static int coroutine_fn
vmdk_co_write_zeroes(BlockDriverState *bs,
int ret;
BDRVVmdkState *s = bs-opaque;
qemu_co_mutex_lock(s-lock);
+/* write zeroes could fail if sectors not aligned to cluster, test it with
+ * dry_run == true before really updating image */
ret = vmdk_write(bs, sector_num, NULL, nb_sectors, true, true);
if (!ret) {
ret = vmdk_write(bs, sector_num, NULL, nb_sectors, true, false);
--
1.8.3.4
Re: [Qemu-devel] [Qemu-trivial] [PATCH v3] block/iscsi.c: Fix printf format error.
On Wed, Jul 31, 2013 at 10:20:26PM +0100, Richard W.M. Jones wrote: From: Richard W.M. Jones rjo...@redhat.com The error on armv7hl was: block/iscsi.c: In function ‘is_request_lun_aligned’: block/iscsi.c:251:26: error: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘int64_t’ [-Werror=format=] iscsilun-block_size, sector_num, nb_sectors); ^ This also splits the long line to comply with qemu coding guidelines. Signed-off-by: Richard W.M. Jones rjo...@redhat.com --- block/iscsi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
[Qemu-devel] [PATCH] target-mips: fix 34Kf configuration for DSP ASE
34Kf core does support DSP ASE. CP0_Config3 configuration for 34Kf and description are wrong. Please refer to MIPS32(R) 34Kf(TM) Processor Core Datasheet Signed-off-by: Yongbok Kim yongbok@imgtec.com --- target-mips/translate_init.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index 7cf238f..4cd9ed5 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -274,14 +274,13 @@ static const mips_def_t mips_defs[] = (0 CP0C1_DS) | (3 CP0C1_DL) | (1 CP0C1_DA) | (1 CP0C1_CA), .CP0_Config2 = MIPS_CONFIG2, -.CP0_Config3 = MIPS_CONFIG3 | (1 CP0C3_VInt) | (1 CP0C3_MT), +.CP0_Config3 = MIPS_CONFIG3 | (1 CP0C3_VInt) | (1 CP0C3_MT) | + (1 CP0C3_DSPP), .CP0_LLAddr_rw_bitmask = 0, .CP0_LLAddr_shift = 0, .SYNCI_Step = 32, .CCRes = 2, -/* No DSP implemented. */ .CP0_Status_rw_bitmask = 0x3678FF1F, -/* No DSP implemented. */ .CP0_TCStatus_rw_bitmask = (0 CP0TCSt_TCU3) | (0 CP0TCSt_TCU2) | (1 CP0TCSt_TCU1) | (1 CP0TCSt_TCU0) | (0 CP0TCSt_TMX) | (1 CP0TCSt_DT) | -- 1.7.4
Re: [Qemu-devel] Using virtio-mmio
On Thu, Aug 01, 2013 at 10:32:14AM +0100, Peter Maydell wrote: On 31 July 2013 23:45, Richard W.M. Jones rjo...@redhat.com wrote: ~/d/qemu/arm-softmmu/qemu-system-arm \ -m 512 -M vexpress-a9 -machine kernel_irqchip=on \ The combination of 'vexpress-a9' and kernel_irqchip=on don't make any sense -- the former implies not using KVM because KVM needs an A15 guest CPU, whereas the latter implies using KVM. In any case kernel_irqchip=on is the default when using KVM. You can just delete the kernel_irqchip option. I did notice that it seemed to do nothing! You might want to consider -M vexpress-a15, if you want a setup that will let you use KVM (will probably need to reconfig your kernel appropriately; may need to discard earlyprintk if you compiled the kernel with the dodgy guess earlyprintk serial port address based on exact revision-and-patchlevel of CPU option.) Unfortunately vexpress-a15 causes the kernel to fail to boot. I guess the Fedora kernel is not configured to work with A15 yet. Thanks, Rich. $ grep VEXPRESS config-3.9.9-302.fc19.armv7hl CONFIG_ARCH_VEXPRESS=y CONFIG_ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA=y CONFIG_ARCH_VEXPRESS_CA9X4=y CONFIG_SENSORS_VEXPRESS=m CONFIG_VEXPRESS_CONFIG=y CONFIG_REGULATOR_VEXPRESS=m -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
Re: [Qemu-devel] Using virtio-mmio
On 1 August 2013 11:41, Richard W.M. Jones rjo...@redhat.com wrote: On Thu, Aug 01, 2013 at 10:32:14AM +0100, Peter Maydell wrote: You might want to consider -M vexpress-a15, if you want a setup that will let you use KVM (will probably need to reconfig your kernel appropriately; may need to discard earlyprintk if you compiled the kernel with the dodgy guess earlyprintk serial port address based on exact revision-and-patchlevel of CPU option.) Unfortunately vexpress-a15 causes the kernel to fail to boot. I guess the Fedora kernel is not configured to work with A15 yet. $ grep VEXPRESS config-3.9.9-302.fc19.armv7hl CONFIG_ARCH_VEXPRESS=y CONFIG_ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA=y CONFIG_ARCH_VEXPRESS_CA9X4=y CONFIG_SENSORS_VEXPRESS=m CONFIG_VEXPRESS_CONFIG=y CONFIG_REGULATOR_VEXPRESS=m Actually the vexpress-A15 doesn't need any extra config switches the way the vexpress-A9 does, because it's all device-tree controlled. Unfortunately at this point you run into the classic issue of trying to get an ARM kernel running, which is that a huge class of config errors all have the failure mode just sits there with no serial output. This is remarkably user-unfriendly but I don't really know how we could fix it since the underlying cause is that serial ports on ARM aren't in a single standard location, so if the kernel's not configured right it won't find the serial port. I usually end up resorting to running with gdb connected to qemu's debug stub and fishing the kernel output out of its log_buf :-( -- PMM
Re: [Qemu-devel] [PATCH] cpus: use cpu_is_stopped efficiently
On 07/26/2013 04:47 PM, Tiejun Chen wrote:
It makes more sense and simple later.
Any feedback :)
Tiejun
Signed-off-by: Tiejun Chen tiejun.c...@windriver.com
---
cpus.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cpus.c b/cpus.c
index c232265..a997632 100644
--- a/cpus.c
+++ b/cpus.c
@@ -62,6 +62,11 @@
static CPUArchState *next_cpu;
+bool cpu_is_stopped(CPUState *cpu)
+{
+return !runstate_is_running() || cpu-stopped;
+}
+
static bool cpu_thread_is_idle(CPUArchState *env)
{
CPUState *cpu = ENV_GET_CPU(env);
@@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
if (cpu-stop || cpu-queued_work_first) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return true;
}
if (!cpu-halted || qemu_cpu_has_work(cpu) ||
@@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
}
}
-bool cpu_is_stopped(CPUState *cpu)
-{
-return !runstate_is_running() || cpu-stopped;
-}
-
static void do_vm_stop(RunState state)
{
if (runstate_is_running()) {
@@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
if (cpu-stop) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return false;
}
return true;
Re: [Qemu-devel] [PATCH] cpus: use cpu_is_stopped efficiently
Hi,
Am 26.07.2013 10:47, schrieb Tiejun Chen:
It makes more sense and simple later.
Signed-off-by: Tiejun Chen tiejun.c...@windriver.com
---
cpus.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cpus.c b/cpus.c
index c232265..a997632 100644
--- a/cpus.c
+++ b/cpus.c
@@ -62,6 +62,11 @@
static CPUArchState *next_cpu;
+bool cpu_is_stopped(CPUState *cpu)
+{
+return !runstate_is_running() || cpu-stopped;
+}
+
static bool cpu_thread_is_idle(CPUArchState *env)
{
CPUState *cpu = ENV_GET_CPU(env);
To optimize performance slightly, I would suggest to reorder the two
conditions as they were below (avoiding the non-inline function call if
cpu-stopped).
Other than that it looks good to me, but no bugfix for 1.6.
If you send a v2 I can queue it on qom-cpu for the next merge window in
two weeks.
CC'ing me would have made me review it earlier. ;) And as you may have
noticed, Avi is no longer with Red Hat, and Gleb and Paolo are
maintaining KVM parts, which there are none in this patch. See
MAINTAINERS file for the latest list.
Regards,
Andreas
@@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
if (cpu-stop || cpu-queued_work_first) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return true;
}
if (!cpu-halted || qemu_cpu_has_work(cpu) ||
@@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
}
}
-bool cpu_is_stopped(CPUState *cpu)
-{
-return !runstate_is_running() || cpu-stopped;
-}
-
static void do_vm_stop(RunState state)
{
if (runstate_is_running()) {
@@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
if (cpu-stop) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return false;
}
return true;
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event
On Thu, Aug 01, 2013 at 11:59:14AM +0300, Michael S. Tsirkin wrote: On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote: netclient 'name' entry in event is useful for management to know which device is changed. n-netclient_name is not always set. This patch changes to use nc-name. If we don't assign 'id', qemu will set a generated name to nc-name. Signed-off-by: Amos Kong ak...@redhat.com Just making sure - you don't think this patch is necessary, correct? Correct.
Re: [Qemu-devel] [RFC] [PATCHv4 01/13] aio / timers: add qemu-timer.c utility functions
On Jul 26 2013, Alex Bligh wrote:
Add qemu_free_clock and expose qemu_new_clock and clock types.
Add utility functions to qemu-timer.c for nanosecond timing.
Add qemu_clock_deadline_ns to calculate deadlines to
nanosecond accuracy.
Add utility function qemu_soonest_timeout to calculate soonest deadline.
Add qemu_timeout_ns_to_ms to convert a timeout in nanoseconds back to
milliseconds for when ppoll is not used.
Signed-off-by: Alex Bligh a...@alex.org.uk
---
include/qemu/timer.h | 17 ++
qemu-timer.c | 63
+-
2 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9dd206c..6171db3 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -11,6 +11,10 @@
#define SCALE_US 1000
#define SCALE_NS 1
+#define QEMU_CLOCK_REALTIME 0
+#define QEMU_CLOCK_VIRTUAL 1
+#define QEMU_CLOCK_HOST 2
+
typedef struct QEMUClock QEMUClock;
typedef void QEMUTimerCB(void *opaque);
@@ -32,10 +36,14 @@ extern QEMUClock *vm_clock;
the virtual clock. */
extern QEMUClock *host_clock;
+QEMUClock *qemu_new_clock(int type);
+void qemu_free_clock(QEMUClock *clock);
int64_t qemu_get_clock_ns(QEMUClock *clock);
int64_t qemu_clock_has_timers(QEMUClock *clock);
int64_t qemu_clock_expired(QEMUClock *clock);
int64_t qemu_clock_deadline(QEMUClock *clock);
+int64_t qemu_clock_deadline_ns(QEMUClock *clock);
+int qemu_timeout_ns_to_ms(int64_t ns);
void qemu_clock_enable(QEMUClock *clock, bool enabled);
void qemu_clock_warp(QEMUClock *clock);
@@ -63,6 +71,15 @@ int64_t cpu_get_ticks(void);
void cpu_enable_ticks(void);
void cpu_disable_ticks(void);
+static inline int64_t qemu_soonest_timeout(int64_t timeout1, int64_t
timeout2)
+{
+/* we can abuse the fact that -1 (which means infinite) is a maximal
+ * value when cast to unsigned. As this is disgusting, it's kept in
+ * one inline function.
+ */
+return ((uint64_t) timeout1 (uint64_t) timeout2) ? timeout1 : timeout2;
+}
+
It becomes much less disgusting if timeouts are made unsigned. I agree
we can do it later.
static inline QEMUTimer *qemu_new_timer_ns(QEMUClock *clock, QEMUTimerCB *cb,
void *opaque)
{
diff --git a/qemu-timer.c b/qemu-timer.c
index b2d95e2..3dfbdbf 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -40,10 +40,6 @@
/***/
/* timers */
-#define QEMU_CLOCK_REALTIME 0
-#define QEMU_CLOCK_VIRTUAL 1
-#define QEMU_CLOCK_HOST 2
-
struct QEMUClock {
QEMUTimer *active_timers;
@@ -231,7 +227,7 @@ QEMUClock *rt_clock;
QEMUClock *vm_clock;
QEMUClock *host_clock;
-static QEMUClock *qemu_new_clock(int type)
+QEMUClock *qemu_new_clock(int type)
{
QEMUClock *clock;
@@ -243,6 +239,11 @@ static QEMUClock *qemu_new_clock(int type)
return clock;
}
+void qemu_free_clock(QEMUClock *clock)
+{
+g_free(clock);
+}
+
void qemu_clock_enable(QEMUClock *clock, bool enabled)
{
bool old = clock-enabled;
@@ -268,7 +269,7 @@ int64_t qemu_clock_deadline(QEMUClock *clock)
/* To avoid problems with overflow limit this to 2^32. */
int64_t delta = INT32_MAX;
-if (clock-active_timers) {
+if (clock-enabled clock-active_timers) {
delta = clock-active_timers-expire_time - qemu_get_clock_ns(clock);
}
if (delta 0) {
@@ -277,6 +278,56 @@ int64_t qemu_clock_deadline(QEMUClock *clock)
return delta;
}
+/*
+ * As above, but return -1 for no deadline, and do not cap to 2^32
+ * as we know the result is always positive.
+ */
+
+int64_t qemu_clock_deadline_ns(QEMUClock *clock)
+{
+int64_t delta;
+
+if (!clock-enabled || !clock-active_timers) {
+return -1;
+}
+
+delta = clock-active_timers-expire_time - qemu_get_clock_ns(clock);
+
+if (delta = 0) {
+return 0;
+}
+
+return delta;
+}
+
+/* Transition function to convert a nanosecond timeout to ms
+ * This is used where a system does not support ppoll
+ */
+int qemu_timeout_ns_to_ms(int64_t ns)
+{
+int64_t ms;
+if (ns 0) {
+return -1;
+}
+
+if (!ns) {
+return 0;
+}
+
+/* Always round up, because it's better to wait too long than to wait too
+ * little and effectively busy-wait
+ */
+ms = (ns + SCALE_MS - 1) / SCALE_MS;
+
+/* To avoid overflow problems, limit this to 2^31, i.e. approx 25 days */
+if (ms (int64_t) INT32_MAX) {
+ms = INT32_MAX;
+}
+
+return (int) ms;
+}
+
+
QEMUTimer *qemu_new_timer(QEMUClock *clock, int scale,
QEMUTimerCB *cb, void *opaque)
{
--
1.7.9.5
Re: [Qemu-devel] [PATCH v3 0/7] Implement reference count for BlockDriverState
On Wed, Jul 31, 2013 at 06:13:53PM +0800, Fam Zheng wrote: BlockDriverState lifecycle management is needed by future features such as image fleecing and blockdev-add. This series adds reference count to BlockDriverState. The first two patches clean up two odd BlockDriverState use cases, so all code uses bdrv_new() to create BlockDriverState instance. Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially, refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach, block-migration and nbd. The rule is: Either bdrv_ref() or bdrv_new() must have a matching bdrv_unref() call, and the last matching bdrv_unref deletes the bs. v3: 03: Removed unnecessary bdrv_close() call. v2: 05: Removed: block: use BlockDriverState refcnt for device attach/detach 07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt. Fam Zheng (7): vvfat: use bdrv_new() to allocate BlockDriverState iscsi: use bdrv_new() instead of stack structure block: implement reference count for BlockDriverState block: make bdrv_delete() static migration: omit drive ref as we have bdrv_ref now xen_disk: simplify blk_disconnect with refcnt nbd: use BlockDriverState refcnt block-migration.c | 4 ++-- block.c | 44 +--- block/backup.c| 2 +- block/blkverify.c | 4 ++-- block/cow.c | 2 +- block/iscsi.c | 14 +++--- block/mirror.c| 2 +- block/qcow.c | 2 +- block/qcow2.c | 2 +- block/qed.c | 2 +- block/sheepdog.c | 6 +++--- block/snapshot.c | 2 +- block/stream.c| 2 +- block/vmdk.c | 10 +- block/vvfat.c | 6 +++--- blockdev-nbd.c| 10 +- blockdev.c| 14 +++--- hw/block/xen_disk.c | 13 ++--- include/block/block.h | 3 ++- include/block/block_int.h | 1 + nbd.c | 5 + qemu-img.c| 26 +- qemu-io.c | 6 +++--- 23 files changed, 101 insertions(+), 81 deletions(-) One interesting thing is the interaction between the DriveInfo and BDS lifecycle. Both now have refcounts but drive_init()/drive_uninit() do bdrv_ref()/bdrv_unref() so you can be sure that the BDS will not go away if you hold a DriveInfo reference. Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
Re: [Qemu-devel] Using virtio-mmio
On Thu, Aug 01, 2013 at 11:58:15AM +0100, Peter Maydell wrote: Unfortunately at this point you run into the classic issue of trying to get an ARM kernel running, which is that a huge class of config errors all have the failure mode just sits there with no serial output. This is remarkably user-unfriendly but I don't really know how we could fix it since the underlying cause is that serial ports on ARM aren't in a single standard location, so if the kernel's not configured right it won't find the serial port. I usually end up resorting to running with gdb connected to qemu's debug stub and fishing the kernel output out of its log_buf :-( Yeah, probably I should fix virt-dmesg to work on ARM guests ... I don't know if there's a better list for asking these questions, but here's another one: ~/d/qemu/arm-softmmu/qemu-system-arm -m 512 -M vexpress-a9 \ -kernel kernel -initrd initrd \ -drive if=none,file=root,id=foo -device virtio-blk-device,drive=foo \ -append root=/dev/vda console=ttyAMA0 rootwait debug -serial stdio This command boots the kernel, loads the initrd, and loads the virtio modules, but the 'root' disk is never enumerated (see full messages attached). I wonder if I'm missing a kernel module for virtio-blk to work? The code that is running in the guest is here if you're interested: https://github.com/libguestfs/supermin/blob/master/helper/init.c --- I'd _really_ prefer to use virtio-scsi, since virtio-blk has all sorts of shortcomings. However I could work out the command line fu to enable virtio-scsi. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ oss: Could not initialize DAC oss: Failed to open `/dev/dsp' oss: Reason: No such file or directory oss: Could not initialize DAC oss: Failed to open `/dev/dsp' oss: Reason: No such file or directory audio: Failed to create voice `lm4549.out' [0.00] Booting Linux on physical CPU 0x0 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Linux version 3.9.9-302.fc19.armv7hl (mockbu...@arm04-builder00.arm.fedoraproject.org) (gcc version 4.8.1 20130603 (Red Hat 4.8.1-1) (GCC) ) #1 SMP Sun Jul 7 02:30:19 UTC 2013 [0.00] CPU: ARMv7 Processor [410fc090] revision 0 (ARMv7), cr=10c5387d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine: ARM-Versatile Express [0.00] Memory policy: ECC disabled, Data cache writealloc [0.00] On node 0 totalpages: 131072 [0.00] free_area_init_node: node 0, pgdat c08af680, node_mem_map c09a3000 [0.00] Normal zone: 1024 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 131072 pages, LIFO batch:31 [0.00] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps every 178956ms [0.00] PERCPU: Embedded 9 pages/cpu @c0da7000 s13056 r8192 d15616 u36864 [0.00] pcpu-alloc: s13056 r8192 d15616 u36864 alloc=9*4096 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 130048 [0.00] Kernel command line: root=/dev/vda console=ttyAMA0 rootwait debug [0.00] PID hash table entries: 2048 (order: 1, 8192 bytes) [0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes) [0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes) [0.00] __ex_table already sorted, skipping sort [0.00] allocated 1048576 bytes of page_cgroup [0.00] please try 'cgroup_disable=memory' option if you don't want memory cgroups [0.00] Memory: 512MB = 512MB total [0.00] Memory: 508080k/508080k available, 16208k reserved, 0K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xe080 - 0xff00 ( 488 MB) [0.00] lowmem : 0xc000 - 0xe000 ( 512 MB) [0.00] pkmap : 0xbfe0 - 0xc000 ( 2 MB) [0.00] modules : 0xbf00 - 0xbfe0 ( 14 MB) [0.00] .text : 0xc0008000 - 0xc07994bc (7750 kB) [0.00] .init : 0xc079a000 - 0xc0817300 ( 501 kB) [0.00] .data : 0xc0818000 - 0xc08bcf30 ( 660 kB) [0.00].bss : 0xc08bcf30 - 0xc09a2c48 ( 920 kB) [0.00] SLUB: Genslabs=11, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 [0.00] Hierarchical RCU implementation. [0.00] RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=1. [0.00] NR_IRQS:16 nr_irqs:16 16 [0.00] GIC CPU mask not found - kernel will fail to boot. [0.00] GIC CPU mask not
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
True, qemu_event basically works only when a single thread resets it. But there is no race condition here because qemu_run_timers cannot be executed concurrently by multiple threads (like aio_poll in your bottom half patches). ... or, if rebasing on top of my patches, qemu_run_timers *can* be executed concurrently by mulitple threads, but in respect of any given QEMUTimerList, it will only be executed by one thread. ... so the event would be placed in QEMUTimerList, not QEMUClock. qemu_clock_enable then will have to visit all timer lists. That's not hard to do, but as locks proliferate we need to have a clear policy (e.g. BQL - clock - timerlist). So actually there is another problem with this patch (both the condvar and the event approach are equally buggy). If a timer on clock X disables clock X, qemu_clock_enable will deadlock. I suppose it's easier to ask each user of qemu_clock_enable to provide its own synchronization, and drop this patch. The simplest kind of synchronization is to delay some work to a bottom half in the clock's AioContext. If you do that, you know that the timers are not running. Ping Fan, this teaches once more the same lesson: let's not invent complex concurrency mechanisms unless really necessary. So far they almost never survived (there are some exceptions, but we have always taken them from somewhere else: QemuEvent is an abstraction of liburcu code to make it portable, RCU and seqlock are from Linux, and I cannot think of anything else). Paolo
Re: [Qemu-devel] [PATCH v3 0/7] Implement reference count for BlockDriverState
On Wed, Jul 31, 2013 at 06:13:53PM +0800, Fam Zheng wrote: BlockDriverState lifecycle management is needed by future features such as image fleecing and blockdev-add. This series adds reference count to BlockDriverState. The first two patches clean up two odd BlockDriverState use cases, so all code uses bdrv_new() to create BlockDriverState instance. Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially, refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach, block-migration and nbd. The rule is: Either bdrv_ref() or bdrv_new() must have a matching bdrv_unref() call, and the last matching bdrv_unref deletes the bs. v3: 03: Removed unnecessary bdrv_close() call. v2: 05: Removed: block: use BlockDriverState refcnt for device attach/detach 07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt. Fam Zheng (7): vvfat: use bdrv_new() to allocate BlockDriverState iscsi: use bdrv_new() instead of stack structure block: implement reference count for BlockDriverState block: make bdrv_delete() static migration: omit drive ref as we have bdrv_ref now xen_disk: simplify blk_disconnect with refcnt nbd: use BlockDriverState refcnt Follow-up question: Did you look at using bdrv_ref() for the BDS - BlockJob relationship too? blockdev.c block job code still uses the DriveInfo refcount after your series. The BDS reference would be sufficient since the DriveInfo fields are not used by block jobs. Stefan
Re: [Qemu-devel] [RFC] [PATCHv4 10/13] aio / timers: Convert mainloop to use timeout
On Jul 26 2013, Alex Bligh wrote:
Convert mainloop to use timeout from 3 static timers.
Signed-off-by: Alex Bligh a...@alex.org.uk
---
main-loop.c | 48 +---
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index a44fff6..c30978b 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -155,10 +155,11 @@ static int max_priority;
static int glib_pollfds_idx;
static int glib_n_poll_fds;
-static void glib_pollfds_fill(uint32_t *cur_timeout)
+static void glib_pollfds_fill(int64_t *cur_timeout)
{
GMainContext *context = g_main_context_default();
int timeout = 0;
+int64_t timeout_ns;
int n;
g_main_context_prepare(context, max_priority);
@@ -174,9 +175,13 @@ static void glib_pollfds_fill(uint32_t *cur_timeout)
glib_n_poll_fds);
} while (n != glib_n_poll_fds);
-if (timeout = 0 timeout *cur_timeout) {
-*cur_timeout = timeout;
+if (timeout 0) {
+timeout_ns = -1;
+} else {
+timeout_ns = (int64_t)timeout * (int64_t)SCALE_MS;
}
+
+*cur_timeout = qemu_soonest_timeout(timeout_ns, *cur_timeout);
}
static void glib_pollfds_poll(void)
@@ -191,7 +196,7 @@ static void glib_pollfds_poll(void)
#define MAX_MAIN_LOOP_SPIN (1000)
-static int os_host_main_loop_wait(uint32_t timeout)
+static int os_host_main_loop_wait(int64_t timeout)
{
int ret;
static int spin_counter;
@@ -214,7 +219,7 @@ static int os_host_main_loop_wait(uint32_t timeout)
notified = true;
}
-timeout = 1;
+timeout = SCALE_MS;
}
if (timeout 0) {
@@ -224,7 +229,7 @@ static int os_host_main_loop_wait(uint32_t timeout)
spin_counter++;
}
-ret = g_poll((GPollFD *)gpollfds-data, gpollfds-len, timeout);
+ret = qemu_poll_ns((GPollFD *)gpollfds-data, gpollfds-len, timeout);
if (timeout 0) {
qemu_mutex_lock_iothread();
@@ -373,7 +378,7 @@ static void pollfds_poll(GArray *pollfds, int nfds,
fd_set *rfds,
}
}
-static int os_host_main_loop_wait(uint32_t timeout)
+static int os_host_main_loop_wait(int64_t timeout)
{
GMainContext *context = g_main_context_default();
GPollFD poll_fds[1024 * 2]; /* this is probably overkill */
@@ -382,6 +387,7 @@ static int os_host_main_loop_wait(uint32_t timeout)
PollingEntry *pe;
WaitObjects *w = wait_objects;
gint poll_timeout;
+int64_t poll_timeout_ns;
static struct timeval tv0;
fd_set rfds, wfds, xfds;
int nfds;
@@ -419,12 +425,17 @@ static int os_host_main_loop_wait(uint32_t timeout)
poll_fds[n_poll_fds + i].events = G_IO_IN;
}
-if (poll_timeout 0 || timeout poll_timeout) {
-poll_timeout = timeout;
+if (poll_timeout 0) {
+poll_timeout_ns = -1;
+} else {
+poll_timeout_ns = (int64_t)poll_timeout * (int64_t)SCALE_MS;
}
+poll_timeout_ns = qemu_soonest_timeout(poll_timeout_ns, timeout);
+
qemu_mutex_unlock_iothread();
-g_poll_ret = g_poll(poll_fds, n_poll_fds + w-num, poll_timeout);
+g_poll_ret = qemu_poll_ns(poll_fds, n_poll_fds + w-num,
poll_timeout_ns);
+
qemu_mutex_lock_iothread();
if (g_poll_ret 0) {
for (i = 0; i w-num; i++) {
@@ -449,6 +460,7 @@ int main_loop_wait(int nonblocking)
{
int ret;
uint32_t timeout = UINT32_MAX;
+int64_t timeout_ns;
if (nonblocking) {
timeout = 0;
@@ -462,7 +474,21 @@ int main_loop_wait(int nonblocking)
slirp_pollfds_fill(gpollfds);
#endif
qemu_iohandler_fill(gpollfds);
-ret = os_host_main_loop_wait(timeout);
+
+if (timeout == UINT32_MAX) {
+timeout_ns = -1;
+} else {
+timeout_ns = (uint64_t)timeout * (int64_t)(SCALE_MS);
+}
+
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(rt_clock));
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(vm_clock));
This must not be included if use_icount.
Allowing only one rt_clock clock for each AioContext is a simplification,
but I'm worried that it will be a problem later. For example, the block
layer wants to use vm_clock. Perhaps QEMUTimerList should really have
three lists, one for each clock type?
Once you do this, you get some complications due to more data structures,
but other code is simplified noticeably. For example, you lose the concept
of a default timerlist (it's just the timerlist of the default AioContext).
And because all timerlists have an AioContext, you do not need to special
case aio_notify() vs. qemu_notify_event().
There are a couple of places to be careful about, of course. For example,
if (use_icount qemu_clock_deadline(vm_clock) = 0) {
Re: [Qemu-devel] [PATCH 2/2] KVM: s390: add floating irq controller
Am 2013 8 1 07:21 schrieb Heiko Carstens heiko.carst...@de.ibm.com:
On Wed, Jul 31, 2013 at 11:08:15AM +0200, Cornelia Huck wrote:
On Mon, 29 Jul 2013 15:59:53 +0200
Jens Freimann jf...@linux.vnet.ibm.com wrote:
This patch adds a floating irq controller as a kvm_device.
It will be necesary for migration of floating interrupts as well
as for hardening the reset code by allowing user space to explicitly
remove all pending floating interrupts.
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
arch/s390/include/uapi/asm/kvm.h | 5 +
arch/s390/kvm/interrupt.c| 210
+++
arch/s390/kvm/kvm-s390.c | 1 +
include/linux/kvm_host.h | 1 +
include/uapi/linux/kvm.h | 1 +
virt/kvm/kvm_main.c | 3 +
6 files changed, 181 insertions(+), 40 deletions(-)
+static int flic_set_attr(struct kvm_device *dev, struct
kvm_device_attr *attr)
+{
+ int r;
+ struct kvm_s390_irq *s390irq;
+ struct kvm_s390_interrupt_info *inti;
+
+ switch (attr-group) {
+ case KVM_DEV_FLIC_ENQUEUE:
+ inti = kzalloc(sizeof(*inti), GFP_KERNEL);
+ if (!inti) {
+ r = -ENOMEM;
+ goto out;
+ }
+ s390irq = kzalloc(sizeof(*s390irq), GFP_KERNEL);
+ if (!s390irq) {
+ r = -ENOMEM;
+ goto out_free_inti;
+ }
+ if (copy_from_user(s390irq, (u64 __user *)attr-addr,
+ sizeof(s390irq))) {
+ r = -EFAULT;
+ goto out_free_s390irq;
+ }
Can't you just copy into inti-irq here? You'd get rid of allocating
two structures that way.
+ switch(s390irq-type) {
+ case KVM_S390_INT_VIRTIO:
+ case KVM_S390_INT_SERVICE:
+ case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
+ case KVM_S390_MCHK:
+ inti-irq = *s390irq;
+ __inject_vm(dev-kvm, inti);
+ default:
+ r = -EINVAL;
+ }
+ break;
And, due to missing break statement, it will always return -EINVAL ;)
I realized that after it was sent out already and told Christian to fix it
before sending the patch further upstream. But thanks anyway :)
I'll send a corrected version as soon as I can find a stable internet
connection here :)
Regards
Jens
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Qemu-devel] pvpanic device should not be automatically included as an internal device
Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Now what will happen on a cluster with hundreds of such VMs? If that cluster has a health monitoring service it may show all the VMs in a not healthy state. My point is that a device that requires a driver that is not inbox, should not be present by default. One possible solution is to add it manually with -device from command line. Any thoughts? Marcel
Re: [Qemu-devel] [PATCH v2 2/2] kvm: migrate vPMU state
On Thu, Aug 01, 2013 at 03:03:12PM +0200, Paolo Bonzini wrote:
KVM disabled HW counters when outside of a guest mode (otherwise result
will be useless), so I do not see how the problem you describe can
happen.
Yes, you're right.
On the other hand MPU emulation assumes that counter have to be disabled
while MSR_IA32_PERFCTR0 is written since write to MSR_IA32_PERFCTR0 does
not reprogram perf evens, so we need either disable/enable counters to
write MSR_IA32_PERFCTR0 or have this patch in the kernel:
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 5c4f631..bf14e42 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -412,6 +412,8 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct
msr_data *msr_info)
if (!msr_info-host_initiated)
data = (s64)(s32)data;
pmc-counter += data - read_pmc(pmc);
+ if (msr_info-host_initiated)
+ reprogram_gp_counter(pmc, pmc-eventsel);
return 0;
} else if ((pmc = get_gp_pmc(pmu, index, MSR_P6_EVNTSEL0))) {
if (data == pmc-eventsel)
Why do you need if (msr_info-host_initiated)? I could not find any
hint in the manual that the overflow counter will still use the value
of the counter that was programmed first.
Not sure I understand. What overflow counter will still use the value
of the counter that was programmed first means?
Strictly speaking we do need if (msr_info-host_initiated) here,
there is no harm in calling reprogram_gp_counter() unconditionally,
but spec says in no vague terms that counter should be disabled before
writing into the MSR and it means that reprogram_gp_counter() will be
called again when guest will enable counter later, so the invocation
here is redundant and since during profiling this happens a lot avoiding
call to reprogram_gp_counter() is a win.
If we need to do it always, I agree it's better to modify the QEMU
patch and not disable/enable the counters. But if we need to restrict
it to host-initiated writes, I would rather have the QEMU patch as I
posted it. So far we always had less side-effects from host_initiated,
not more, and I think it's a good rule of thumb.
I am OK with your patch, it is a little bit unfortunate that userspase
should care about such low level details though.
--
Gleb.
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Paolo, --On 1 August 2013 08:19:34 -0400 Paolo Bonzini pbonz...@redhat.com wrote: True, qemu_event basically works only when a single thread resets it. But there is no race condition here because qemu_run_timers cannot be executed concurrently by multiple threads (like aio_poll in your bottom half patches). ... or, if rebasing on top of my patches, qemu_run_timers *can* be executed concurrently by mulitple threads, but in respect of any given QEMUTimerList, it will only be executed by one thread. ... so the event would be placed in QEMUTimerList, not QEMUClock. Correct qemu_clock_enable then will have to visit all timer lists. That's not hard to do, Correct, v4 of my patch does this. but as locks proliferate we need to have a clear policy (e.g. BQL - clock - timerlist). But doesn't do the locking bit yet (Pingfan is going to do that I hope) So actually there is another problem with this patch (both the condvar and the event approach are equally buggy). If a timer on clock X disables clock X, qemu_clock_enable will deadlock. Yes. I believe there will be a similar problem if a timer created or deleted an AioContext (clearly less likely!) I suppose it's easier to ask each user of qemu_clock_enable to provide its own synchronization, and drop this patch. The simplest kind of synchronization is to delay some work to a bottom half in the clock's AioContext. If you do that, you know that the timers are not running. I'm not sure that's true. If two AioContexts run in different threads, would their BH's and timers not also run in those two different threads? -- Alex Bligh
Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event
Am 01.07.2013 04:55, schrieb Amos Kong: On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote: Amos Kong ak...@redhat.com writes: On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote: netclient 'name' entry in event is useful for management to know which device is changed. n-netclient_name is not always set. This patch changes to use nc-name. If we don't assign 'id', qemu will set a generated name to nc-name. IRC: mst akong, what do other events include? name or id? I just checked QMP/qmp-event.txt, they all use 'device name'. (eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*) If we assign 'id' for -device, device name will be set to id. Otherwise, a generated device name will set to some device. DEVICE_DELETED uses device (the qdev ID) and path (the QOM path). For reasons I don't understand, it sets path only when the device has no qdev ID. That should be cleaned up. The path are alwasy set. example: (have id) path: /machine/peripheral-anon/vnet0/virtio-backend You hopefully meant /machine/peripheral/vnet0/virtio-backend? Otherwise we have a bug somewhere. Andreas (no id) path: /machine/peripheral-anon/device[0]/virtio-backend It's enough to just use path to distinguish the changed device. So we ignore this patch. -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2 2/2] kvm: migrate vPMU state
On Aug 01 2013, Gleb Natapov wrote:
On Thu, Aug 01, 2013 at 03:03:12PM +0200, Paolo Bonzini wrote:
KVM disabled HW counters when outside of a guest mode (otherwise result
will be useless), so I do not see how the problem you describe can
happen.
Yes, you're right.
On the other hand MPU emulation assumes that counter have to be disabled
while MSR_IA32_PERFCTR0 is written since write to MSR_IA32_PERFCTR0 does
not reprogram perf evens, so we need either disable/enable counters to
write MSR_IA32_PERFCTR0 or have this patch in the kernel:
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 5c4f631..bf14e42 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -412,6 +412,8 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct
msr_data *msr_info)
if (!msr_info-host_initiated)
data = (s64)(s32)data;
pmc-counter += data - read_pmc(pmc);
+ if (msr_info-host_initiated)
+ reprogram_gp_counter(pmc, pmc-eventsel);
return 0;
} else if ((pmc = get_gp_pmc(pmu, index, MSR_P6_EVNTSEL0))) {
if (data == pmc-eventsel)
Why do you need if (msr_info-host_initiated)? I could not find any
hint in the manual that the overflow counter will still use the value
of the counter that was programmed first.
Not sure I understand. What overflow counter will still use the value
of the counter that was programmed first means?
spec says in no vague terms that counter should be disabled before
writing into the MSR and it means that reprogram_gp_counter() will be
called again when guest will enable counter later,
Yeah, I found it now.
I am OK with your patch, it is a little bit unfortunate that userspase
should care about such low level details though.
Is it a Reviewed-by?
Paolo
Re: [Qemu-devel] [PATCH v2 2/2] kvm: migrate vPMU state
On Thu, Aug 01, 2013 at 03:48:29PM +0200, Paolo Bonzini wrote:
On Aug 01 2013, Gleb Natapov wrote:
On Thu, Aug 01, 2013 at 03:03:12PM +0200, Paolo Bonzini wrote:
KVM disabled HW counters when outside of a guest mode (otherwise result
will be useless), so I do not see how the problem you describe can
happen.
Yes, you're right.
On the other hand MPU emulation assumes that counter have to be disabled
while MSR_IA32_PERFCTR0 is written since write to MSR_IA32_PERFCTR0 does
not reprogram perf evens, so we need either disable/enable counters to
write MSR_IA32_PERFCTR0 or have this patch in the kernel:
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 5c4f631..bf14e42 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -412,6 +412,8 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct
msr_data *msr_info)
if (!msr_info-host_initiated)
data = (s64)(s32)data;
pmc-counter += data - read_pmc(pmc);
+ if (msr_info-host_initiated)
+ reprogram_gp_counter(pmc,
pmc-eventsel);
return 0;
} else if ((pmc = get_gp_pmc(pmu, index,
MSR_P6_EVNTSEL0))) {
if (data == pmc-eventsel)
Why do you need if (msr_info-host_initiated)? I could not find any
hint in the manual that the overflow counter will still use the value
of the counter that was programmed first.
Not sure I understand. What overflow counter will still use the value
of the counter that was programmed first means?
spec says in no vague terms that counter should be disabled before
writing into the MSR and it means that reprogram_gp_counter() will be
called again when guest will enable counter later,
Yeah, I found it now.
I am OK with your patch, it is a little bit unfortunate that userspase
should care about such low level details though.
Is it a Reviewed-by?
here is one :)
Reviewed-by: Gleb Natapov g...@redhat.com
--
Gleb.
Re: [Qemu-devel] [PATCH] monitor: fix parsing of big int
On 08/01/2013 12:31 AM, Fam Zheng wrote: Fix it by calling strtoll instead, which will report ERANGE as expected. (HMP) block_set_io_throttle ide0-hd0 99 0 0 0 0 0 (HMP) block_set_io_throttle ide0-hd0 999 0 0 0 0 0 number too large (HMP) block_set_io_throttle ide0-hd0 0 0 0 0 0 number too large Your change causes this error message: (HMP) block_set_io_throttle ide0-hd0 - 0 0 0 0 0 number too large Does the too large mean in magnitude (correct message) or in value (misleading message, as any negative number is smaller in value than our minimum of 0)? Signed-off-by: Fam Zheng f...@redhat.com --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 5dc0aa9..7bfb469 100644 --- a/monitor.c +++ b/monitor.c @@ -3286,7 +3286,7 @@ static int64_t expr_unary(Monitor *mon) break; default: errno = 0; -n = strtoull(pch, p, 0); +n = strtoll(pch, p, 0); I'm worried that this will break callers that treat their argument as unsigned, and where the full range of unsigned input was desirable. At this point, it's probably safer to do a case-by-case analysis of all callers that use expr_unary() to decide which callers must reject negative values, instead of making the parser reject numbers that it previously accepted, thus changing the behavior of callers that treated the result as unsigned. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC] [PATCHv4 10/13] aio / timers: Convert mainloop to use timeout
Paolo,
@@ -449,6 +460,7 @@ int main_loop_wait(int nonblocking)
{
int ret;
uint32_t timeout = UINT32_MAX;
+int64_t timeout_ns;
if (nonblocking) {
timeout = 0;
@@ -462,7 +474,21 @@ int main_loop_wait(int nonblocking)
slirp_pollfds_fill(gpollfds);
# endif
qemu_iohandler_fill(gpollfds);
-ret = os_host_main_loop_wait(timeout);
+
+if (timeout == UINT32_MAX) {
+timeout_ns = -1;
+} else {
+timeout_ns = (uint64_t)timeout * (int64_t)(SCALE_MS);
+}
+
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(rt_clock));
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(vm_clock));
This must not be included if use_icount.
Really? qemu_run_all_timers was running all 3 timers irrespective of
use_icount, and doing a qemu_notify if anything expired, so I'm merely
mimicking the existing behaviour here.
I'm not quite sure what use_icount does. Does vm_clock get disabled
if it is set? (in which case it won't matter).
Allowing only one rt_clock clock for each AioContext is a simplification,
but I'm worried that it will be a problem later. For example, the block
layer wants to use vm_clock. Perhaps QEMUTimerList should really have
three lists, one for each clock type?
Well currently each QEMUClock has a default QEMUTimerList, so that
wouldn't work well - see below.
The way it's done at the moment is that the QEMUTimerList user can
create as many QEMUTimerLists as he wants. So AioContext asks for one
of one type. It could equally ask for three - one of each type.
I think that's probably adequate.
Once you do this, you get some complications due to more data structures,
but other code is simplified noticeably. For example, you lose the
concept of a default timerlist (it's just the timerlist of the default
AioContext).
Yep - per the above that's really intrusive (I think I touched well over
a hundred files). The problem is that lots of things refer to vm_clock
to set a timer (when it's a clock so should use a timer list) and
also to vm_clock to read the timer (which is a clock function). Changing
vm_clock to vm_timerlist and vm_clock was truly horrible. Hence the
default timer list concept, which I admit is not great but saved a
horribly intrusive patch not all of which I could test. I did this
patch, and scrapped it.
And because all timerlists have an AioContext,
Well old callers, particularly those not using an AioContext, would
not have an AioContext would they?
you do not
need to special case aio_notify() vs. qemu_notify_event().
Well, if I do a v5, I was going to make the constructor for
creating a timerlist specify a callback function to say what should
happen if the clock is enabled etc., and if none was specified
call qemu_notify_event(). The AioContext user would specify a callback
that called aio_notify(). This would be a bit nicer.
There are a couple of places to be careful about, of course. For example,
if (use_icount qemu_clock_deadline(vm_clock) = 0) {
qemu_notify_event();
}
in cpus.c must be changed to iterate over all timerlists.
I was trying hard to avoid anything having to iterate over all
timerlists, and leave the timerlist to be per-thread where possible.
This may well fail for the clock warp stuff. I probably need to
exactly the same as on qemu_clock_enable() here if use_icount is
true. WDYT?
--
Alex Bligh
[Qemu-devel] qemu virtfs 9p not working on arm?
Hi, I'm not sure whether this is a kernel problem or a qemu problem (or even a user problem). The folks on OFTC #qemu suggested I email the list and relevant section maintainers so that's what I'm doing -- hope it's okay. I'm trying to boot qemu-system-arm with rootfs on 9p, and experiencing the following error: | 9pnet: Installing 9P2000 support | 9pnet_virtio: probe of virtio0 failed with error -2 | VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 4 | rtc-pl031 dev:e8: setting system clock to 2013-07-31 22:56:25 UTC (1375311385) | 9pnet_virtio: no channels available | VFS: Cannot open root device root or unknown-block(0,0): error -2 | Please append a correct root= boot option; here are the available partitions: | 0b00 1048575 sr0 driver: sr | Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) That looks like ENOENT during the probe to me, but I can't find where in the kernel source it's coming from. My configuration is: -append root=root rw rootflags=rw,trans=virtio,version=9p2000.L -rootfstype=9p -fsdev local,id=root,path=$(pwd)/root,security_model=none -device virtio-9p-pci,fsdev=root,mount_tag=root and this same configuration seems to work on x86. The kernels I'm using are from the latest Aboriginal Linux images (3.10) and /proc/config.gz seems to show that all the 9p/virtfs options are enabled. Attempting to mount the filesystem after booting also fails with: | 9pnet_virtio: no channels available | mount: mounting root on /mnt/ failed: No such file or directory I have not tested other targets. Finally, my version of qemu is 1.5.1 from the Debian packages. Rich
[Qemu-devel] [Bug 1207228] [NEW] Qemu (trunk code) crashes when using --soundhw all option in ioport.c
Public bug reported: After not building qemu (git version) for about 3 weeks, I've done it again this morning. With up-to-date trunk code, I got this error on start, when using --soundhw all option $ qemu-system-i386 -soundhw all qemu-system-i386: /home/fred/Téléchargements/logs/qemu-git/src/qemu/ioport.c:240: portio_list_add: Assertion `pio-offset = off_last' failed. Abandon (core dumped) And if I use only soundhw with one or more options, it doesn't crash. Tell me what you'll need to fix this bug. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1207228 Title: Qemu (trunk code) crashes when using --soundhw all option in ioport.c Status in QEMU: New Bug description: After not building qemu (git version) for about 3 weeks, I've done it again this morning. With up-to-date trunk code, I got this error on start, when using --soundhw all option $ qemu-system-i386 -soundhw all qemu-system-i386: /home/fred/Téléchargements/logs/qemu-git/src/qemu/ioport.c:240: portio_list_add: Assertion `pio-offset = off_last' failed. Abandon (core dumped) And if I use only soundhw with one or more options, it doesn't crash. Tell me what you'll need to fix this bug. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1207228/+subscriptions
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
On Aug 01 2013, Alex Bligh wrote: So actually there is another problem with this patch (both the condvar and the event approach are equally buggy). If a timer on clock X disables clock X, qemu_clock_enable will deadlock. Yes. I believe there will be a similar problem if a timer created or deleted an AioContext (clearly less likely!) I suppose it's easier to ask each user of qemu_clock_enable to provide its own synchronization, and drop this patch. The simplest kind of synchronization is to delay some work to a bottom half in the clock's AioContext. If you do that, you know that the timers are not running. I'm not sure that's true. If two AioContexts run in different threads, would their BH's and timers not also run in those two different threads? Suppose a thread wants to do qemu_clock_enable(foo, false), and the code after qemu_clock_enable assumes that no timers are running. Then you should move that code to a bottom half in foo's AioContext. Paolo
[Qemu-devel] [PATCH 3/8] [PATCH RFC v3] s390-qemu: cpu hotplug - SCLP Event integration
From: Jason J. Herne jjhe...@us.ibm.com
Add an sclp event for cpu was hot plugged. This allows Qemu to deliver an
SCLP interrupt to the guest stating that the requested cpu hotplug was
completed.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/Makefile.objs|2 +-
hw/s390x/event-facility.c |7 +++
hw/s390x/sclpcpu.c| 120 +
include/hw/s390x/event-facility.h |3 +
include/hw/s390x/sclp.h |1 +
5 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 hw/s390x/sclpcpu.c
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 77e1218..104ae8e 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -2,7 +2,7 @@ obj-y = s390-virtio-bus.o s390-virtio.o
obj-y += s390-virtio-hcall.o
obj-y += sclp.o
obj-y += event-facility.o
-obj-y += sclpquiesce.o
+obj-y += sclpquiesce.o sclpcpu.o
obj-y += ipl.o
obj-y += css.o
obj-y += s390-virtio-ccw.o
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 0faade0..aec35cb 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -317,6 +317,7 @@ static int init_event_facility(S390SCLPDevice *sdev)
{
SCLPEventFacility *event_facility;
DeviceState *quiesce;
+DeviceState *cpu_hotplug;
event_facility = g_malloc0(sizeof(SCLPEventFacility));
sdev-ef = event_facility;
@@ -335,6 +336,12 @@ static int init_event_facility(S390SCLPDevice *sdev)
}
qdev_init_nofail(quiesce);
+cpu_hotplug = qdev_create(event_facility-sbus.qbus, sclpcpuhotplug);
+if (!cpu_hotplug) {
+return -1;
+}
+qdev_init_nofail(cpu_hotplug);
+
return 0;
}
diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c
new file mode 100644
index 000..5b4139e
--- /dev/null
+++ b/hw/s390x/sclpcpu.c
@@ -0,0 +1,120 @@
+/*
+ * SCLP event type
+ *Signal CPU - Trigger SCLP interrupt for system CPU configure or
+ *de-configure
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ * Thang Pham thang.p...@us.ibm.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version. See the COPYING file in the top-level directory.
+ *
+ */
+#include hw/qdev.h
+#include sysemu/sysemu.h
+#include hw/s390x/sclp.h
+#include hw/s390x/event-facility.h
+#include cpu.h
+#include sysemu/cpus.h
+#include sysemu/kvm.h
+
+typedef struct ConfigMgtData {
+EventBufferHeader ebh;
+uint8_t reserved;
+uint8_t event_qualifier;
+} QEMU_PACKED ConfigMgtData;
+
+static qemu_irq irq_cpu_hotplug; /* Only used in this file */
+
+#define EVENT_QUAL_CPU_CHANGE 1
+
+void raise_irq_cpu_hotplug(void)
+{
+qemu_irq_raise(irq_cpu_hotplug);
+}
+
+static int event_type(void)
+{
+return SCLP_EVENT_CONFIG_MGT_DATA;
+}
+
+static unsigned int send_mask(void)
+{
+return SCLP_EVENT_MASK_CONFIG_MGT_DATA;
+}
+
+static unsigned int receive_mask(void)
+{
+return 0;
+}
+
+static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+ int *slen)
+{
+ConfigMgtData *cdata = (ConfigMgtData *) evt_buf_hdr;
+if (*slen sizeof(ConfigMgtData)) {
+return 0;
+}
+
+/* Event is no longer pending */
+if (!event-event_pending) {
+return 0;
+}
+event-event_pending = false;
+
+/* Event header data */
+cdata-ebh.length = cpu_to_be16(sizeof(ConfigMgtData));
+cdata-ebh.type = SCLP_EVENT_CONFIG_MGT_DATA;
+cdata-ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+
+/* Trigger a rescan of CPUs by setting event qualifier */
+cdata-event_qualifier = EVENT_QUAL_CPU_CHANGE;
+*slen -= sizeof(ConfigMgtData);
+
+return 1;
+}
+
+static void trigger_signal(void *opaque, int n, int level)
+{
+SCLPEvent *event = opaque;
+event-event_pending = true;
+
+/* Trigger SCLP read operation */
+sclp_service_interrupt(0);
+}
+
+static int irq_cpu_hotplug_init(SCLPEvent *event)
+{
+irq_cpu_hotplug = *qemu_allocate_irqs(trigger_signal, event, 1);
+return 0;
+}
+
+static void cpu_class_init(ObjectClass *klass, void *data)
+{
+SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
+
+k-init = irq_cpu_hotplug_init;
+k-get_send_mask = send_mask;
+k-get_receive_mask = receive_mask;
+k-event_type = event_type;
+k-read_event_data = read_event_data;
+k-write_event_data = NULL;
+}
+
+static TypeInfo sclp_cpu_info = {
+.name = sclpcpuhotplug,
+.parent= TYPE_SCLP_EVENT,
+.instance_size = sizeof(SCLPEvent),
+.class_init= cpu_class_init,
+.class_size= sizeof(SCLPEventClass),
+};
+
+static void register_types(void)
+{
+type_register_static(sclp_cpu_info);
+}
+
+type_init(register_types)
+
diff --git a/include/hw/s390x/event-facility.h
b/include/hw/s390x/event-facility.h
index 791ab2a..d63969f 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -17,14 +17,17 @@
[Qemu-devel] [PATCH 1/8] [PATCH RFC v3] s390-qemu: cpu hotplug - Define New SCLP Codes
From: Jason J. Herne jjhe...@us.ibm.com
Define new SCLP codes to improve code readability.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/sclp.c |2 +-
include/hw/s390x/sclp.h |8
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 86d6ae0..cb53d7e 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -45,7 +45,7 @@ static void sclp_execute(SCCB *sccb, uint64_t code)
{
S390SCLPDevice *sdev = get_event_facility();
-switch (code) {
+switch (code SCLP_NO_CMD_PARM) {
case SCLP_CMDW_READ_SCP_INFO:
case SCLP_CMDW_READ_SCP_INFO_FORCED:
read_SCP_info(sccb);
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 231a38a..174097d 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -26,6 +26,14 @@
#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
#define SCLP_CMD_WRITE_EVENT_MASK 0x00780005
+/* CPU hotplug SCLP codes */
+#define SCLP_NO_CMD_PARM0x00ff
+#define SCLP_HAS_CPU_INFO 0x0C00ULL
+#define SCLP_CMDW_READ_CPU_INFO 0x00010001
+#define SCLP_CMDW_CONFIGURE_CPU 0x00110001
+#define SCLP_CMDW_DECONFIGURE_CPU 0x0011
+#define SCLP_CMDW_CPU_CMD_PARM 0xff00
+
/* SCLP response codes */
#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
#define SCLP_RC_NORMAL_COMPLETION 0x0020
--
1.7.10.4
[Qemu-devel] [PATCH 8/8] [PATCH RFC v3] qemu-monitor: HMP cpu-add wrapper
From: Jason J. Herne jjhe...@us.ibm.com
Add HMP cpu-add wrapper to allow cpu hot plugging via monitor.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hmp-commands.hx | 13 +
hmp.c | 10 ++
hmp.h |1 +
3 files changed, 24 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8c6b91a..cb8712b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1587,6 +1587,19 @@ Executes a qemu-io command on the given block device.
ETEXI
{
+.name = cpu-add,
+.args_type = id:i,
+.params = id,
+.help = add cpu,
+.mhandler.cmd = hmp_cpu_add,
+},
+
+STEXI
+@item cpu-add @var{id}
+Add CPU with id @var{id}
+ETEXI
+
+{
.name = info,
.args_type = item:s?,
.params = [subcommand],
diff --git a/hmp.c b/hmp.c
index c45514b..9465bd4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1475,6 +1475,16 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict
*qdict)
hmp_handle_error(mon, errp);
}
+void hmp_cpu_add(Monitor *mon, const QDict *qdict)
+{
+int cpuid;
+Error *err = NULL;
+
+cpuid = qdict_get_int(qdict, id);
+qmp_cpu_add(cpuid, err);
+hmp_handle_error(mon, err);
+}
+
void hmp_chardev_add(Monitor *mon, const QDict *qdict)
{
const char *args = qdict_get_str(qdict, args);
diff --git a/hmp.h b/hmp.h
index 6c3bdcd..9effca5 100644
--- a/hmp.h
+++ b/hmp.h
@@ -87,5 +87,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_cpu_add(Monitor *mon, const QDict *qdict);
#endif
--
1.7.10.4
Re: [Qemu-devel] [RFC] [PATCHv4 10/13] aio / timers: Convert mainloop to use timeout
On Aug 01 2013, Alex Bligh wrote:
Paolo,
@@ -449,6 +460,7 @@ int main_loop_wait(int nonblocking)
{
int ret;
uint32_t timeout = UINT32_MAX;
+int64_t timeout_ns;
if (nonblocking) {
timeout = 0;
@@ -462,7 +474,21 @@ int main_loop_wait(int nonblocking)
slirp_pollfds_fill(gpollfds);
# endif
qemu_iohandler_fill(gpollfds);
-ret = os_host_main_loop_wait(timeout);
+
+if (timeout == UINT32_MAX) {
+timeout_ns = -1;
+} else {
+timeout_ns = (uint64_t)timeout * (int64_t)(SCALE_MS);
+}
+
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(rt_clock));
+timeout_ns = qemu_soonest_timeout(timeout_ns,
+ qemu_clock_deadline_ns(vm_clock));
This must not be included if use_icount.
Really? qemu_run_all_timers was running all 3 timers irrespective of
use_icount, and doing a qemu_notify if anything expired, so I'm merely
mimicking the existing behaviour here.
Maybe I'm misreading the code. If it is a replacement of
qemu_next_alarm_deadline, then it is indeed omitting vm_clock.
I'm not quite sure what use_icount does. Does vm_clock get disabled
if it is set? (in which case it won't matter).
It doesn't count nanoseconds anymore. The clock is updated by the
VCPU thread. When the VCPU thread notices that the clock is past the
earliest timers, it does a qemu_notify_event(). That exits the g_poll
and qemu_run_all_timers then can process the callbacks.
The way it's done at the moment is that the QEMUTimerList user can
create as many QEMUTimerLists as he wants. So AioContext asks for one
of one type. It could equally ask for three - one of each type.
I think that's probably adequate.
Once you do this, you get some complications due to more data structures,
but other code is simplified noticeably. For example, you lose the
concept of a default timerlist (it's just the timerlist of the default
AioContext).
Yep - per the above that's really intrusive (I think I touched well over
a hundred files). The problem is that lots of things refer to vm_clock
to set a timer (when it's a clock so should use a timer list) and
also to vm_clock to read the timer (which is a clock function).
Yes, that's fine. You can still keep the shorter invocation,
but instead of using clock-timerlist it would use
qemu_aio_context-clocks[clock-type].
Related to this, a better name for the full functions taking
a timerlist could be simply timer_new_ns etc. And I would remove
the allocation step for these functions. It is shameful how little
we use qemu_free_timer, and removing allocation would fix the
problem completely for users of the QEMUTimerList-based functions.
It's already a convention to use qemu_* only for functions that use some
global state, for example qemu_notify_event() vs. aio_notify().
And because all timerlists have an AioContext,
Well old callers, particularly those not using an AioContext, would
not have an AioContext would they?
It would be qemu_aio_context.
I was trying hard to avoid anything having to iterate over all
timerlists, and leave the timerlist to be per-thread where possible.
This may well fail for the clock warp stuff. I probably need to
exactly the same as on qemu_clock_enable() here if use_icount is
true. WDYT?
Yes. This:
qemu_mod_timer(icount_warp_timer, vm_clock_warp_start + deadline);
would have to use the earliest deadline of all vm_clock timerlists.
And this:
if (qemu_clock_expired(vm_clock)) {
qemu_notify_event();
}
would also have to walk all timerlists for vm_clock, and notify
those that have expired. But you would not need one warp timer
per timerlist.
Paolo
[Qemu-devel] [PATCH 6/8] [PATCH RFC v3] s390-qemu: cpu hotplug - s390 cpu init improvements for hotplug
From: Jason J. Herne jjhe...@us.ibm.com
s390_new_cpu is created to encapsulate the creation of a new QOM S390CPU
object given a cpuid and a model string.
All actual cpu initialization code is moved from boot time specific
functions to
s390_cpu_initfn (qom init routine) or to s390_new_cpu. This is done to
allow us
to use the same basic code path for a cpu created at boot time and one
created
during a hotplug operation.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/s390-virtio.c | 25 -
target-s390x/cpu.c |4 ++--
target-s390x/cpu.h |1 +
target-s390x/helper.c | 12
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 5ad9cf3..103f32e 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -56,11 +56,16 @@ static S390CPU **ipi_states;
void s390_cpu_set_ipistate(uint16_t cpu_addr, S390CPU *state)
{
-ipi_states[cpu_addr] = state;
+if (cpu_addr max_cpus) {
+ipi_states[cpu_addr] = state;
+}
}
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
{
+if (cpu_addr = max_cpus) {
+return NULL;
+}
return ipi_states[cpu_addr];
}
@@ -197,19 +202,13 @@ void s390_init_cpus(const char *cpu_model)
cpu_model = host;
}
-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
-
-for (i = 0; i smp_cpus; i++) {
-S390CPU *cpu;
-CPUState *cs;
+ipi_states = g_malloc(sizeof(S390CPU *) * max_cpus);
-cpu = cpu_s390x_init(cpu_model);
-cs = CPU(cpu);
-
-ipi_states[i] = cpu;
-cs-halted = 1;
-cpu-env.exception_index = EXCP_HLT;
-cpu-env.storage_keys = s390_get_storage_keys();
+for (i = 0; i max_cpus; i++) {
+ipi_states[i] = NULL;
+if (i smp_cpus) {
+s390_new_cpu(cpu_model, i);
+}
}
}
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 6be6c08..c90a91c 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -116,7 +116,6 @@ static void s390_cpu_initfn(Object *obj)
S390CPU *cpu = S390_CPU(obj);
CPUS390XState *env = cpu-env;
static bool inited;
-static int cpu_num = 0;
#if !defined(CONFIG_USER_ONLY)
struct tm tm;
#endif
@@ -135,8 +134,9 @@ static void s390_cpu_initfn(Object *obj)
* cpu counter in s390_cpu_reset to a negative number at
* initial ipl */
cs-halted = 1;
+cpu-env.exception_index = EXCP_HLT;
+env-storage_keys = s390_get_storage_keys();
#endif
-env-cpu_num = cpu_num++;
env-ext_index = -1;
if (tcg_enabled() !inited) {
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 62eb810..0f68dd0 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -315,6 +315,7 @@ static inline int get_ilen(uint8_t opc)
#endif
S390CPU *cpu_s390x_init(const char *cpu_model);
+S390CPU *s390_new_cpu(const char *cpu_model, int64_t cpuid);
void s390x_translate_init(void);
int cpu_s390x_exec(CPUS390XState *s);
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 61abfd7..a39b148 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -70,6 +70,18 @@ void s390x_cpu_timer(void *opaque)
}
#endif
+S390CPU *s390_new_cpu(const char *cpu_model, int64_t cpuid)
+{
+S390CPU *cpu;
+
+cpu = cpu_s390x_init(cpu_model);
+cpu-env.cpu_num = cpuid;
+#if !defined(CONFIG_USER_ONLY)
+s390_cpu_set_ipistate(cpuid, cpu);
+#endif
+return cpu;
+}
+
S390CPU *cpu_s390x_init(const char *cpu_model)
{
S390CPU *cpu;
--
1.7.10.4
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Paolo, --On 1 August 2013 15:51:11 +0200 Paolo Bonzini pbonz...@redhat.com wrote: So actually there is another problem with this patch (both the condvar and the event approach are equally buggy). If a timer on clock X disables clock X, qemu_clock_enable will deadlock. Yes. I believe there will be a similar problem if a timer created or deleted an AioContext (clearly less likely!) I suppose it's easier to ask each user of qemu_clock_enable to provide its own synchronization, and drop this patch. The simplest kind of synchronization is to delay some work to a bottom half in the clock's AioContext. If you do that, you know that the timers are not running. I'm not sure that's true. If two AioContexts run in different threads, would their BH's and timers not also run in those two different threads? Suppose a thread wants to do qemu_clock_enable(foo, false), and the code after qemu_clock_enable assumes that no timers are running. Then you should move that code to a bottom half in foo's AioContext. foo is a QEMUClock here. A QEMUClock may not have just one AioContext. It could have several each operated by a different thread. -- Alex Bligh
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
On Aug 01 2013, Alex Bligh wrote: Paolo, --On 1 August 2013 15:51:11 +0200 Paolo Bonzini pbonz...@redhat.com wrote: So actually there is another problem with this patch (both the condvar and the event approach are equally buggy). If a timer on clock X disables clock X, qemu_clock_enable will deadlock. Yes. I believe there will be a similar problem if a timer created or deleted an AioContext (clearly less likely!) I suppose it's easier to ask each user of qemu_clock_enable to provide its own synchronization, and drop this patch. The simplest kind of synchronization is to delay some work to a bottom half in the clock's AioContext. If you do that, you know that the timers are not running. I'm not sure that's true. If two AioContexts run in different threads, would their BH's and timers not also run in those two different threads? Suppose a thread wants to do qemu_clock_enable(foo, false), and the code after qemu_clock_enable assumes that no timers are running. Then you should move that code to a bottom half in foo's AioContext. foo is a QEMUClock here. A QEMUClock may not have just one AioContext. It could have several each operated by a different thread. Oops, you're right. Checking the code for callers of qemu_clock_enable() it seems like there shouldn't be any deadlocks. So it should work if qemu_clock_enable walks all of the timerlists and waits on each event. But we should document the expectations since they are different from the current code. Paolo
[Qemu-devel] [PULL 1/2] migration: send total time in QMP at completed stage
From: Pawit Pornkitprasan p.pa...@gmail.com The completed stage sets total_time but not has_total_time and thus it is not sent via QMP reply (but sent via HMP nevertheless) Signed-off-by: Pawit Pornkitprasan p.pa...@gmail.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Orit Wasserman owass...@redhat.com Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- migration.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration.c b/migration.c index 9fc7294..3f682cd 100644 --- a/migration.c +++ b/migration.c @@ -231,6 +231,7 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-has_status = true; info-status = g_strdup(completed); +info-has_total_time = true; info-total_time = s-total_time; info-has_downtime = true; info-downtime = s-downtime; -- 1.8.1.4
[Qemu-devel] [PULL for-1.6 0/2] QMP queue
The following changes since commit 1197cbb9eda1dc82e2fa1815ca62bc3de158353e: qdev: Use clz in print_size (2013-07-31 07:54:21 -0500) are available in the git repository at: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp for you to fetch changes up to 8c0426aed1d2279845e6a2c3355da8b5d9926cb6: migration: don't use uninitialized variables (2013-08-01 09:40:46 -0400) Pawit Pornkitprasan (2): migration: send total time in QMP at completed stage migration: don't use uninitialized variables migration.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 1.8.1.4
[Qemu-devel] [PULL 2/2] migration: don't use uninitialized variables
From: Pawit Pornkitprasan p.pa...@gmail.com
The qmp_migrate method uses the 'blk' and 'inc' parameter without
checking if they're valid or not (they may be uninitialized if
command is received via QMP)
Signed-off-by: Pawit Pornkitprasan p.pa...@gmail.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
migration.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/migration.c b/migration.c
index 3f682cd..1402fa7 100644
--- a/migration.c
+++ b/migration.c
@@ -400,8 +400,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
MigrationParams params;
const char *p;
-params.blk = blk;
-params.shared = inc;
+params.blk = has_blk blk;
+params.shared = has_inc inc;
if (s-state == MIG_STATE_ACTIVE || s-state == MIG_STATE_SETUP) {
error_set(errp, QERR_MIGRATION_ACTIVE);
--
1.8.1.4
Re: [Qemu-devel] [PATCH 0/4] dump-guest-memory: correct the vmcores
On Thu, 1 Aug 2013 09:41:07 -0400 Luiz Capitulino lcapitul...@redhat.com wrote: Applied to the qmp branch, thanks. Hmm, it brakes the build. Dropping it from the queue for now: /home/lcapitulino/work/src/upstream/qmp-unstable/target-s390x/arch_dump.c:179:5: error: conflicting types for ‘cpu_get_dump_info’ int cpu_get_dump_info(ArchDumpInfo *info) ^ In file included from /home/lcapitulino/work/src/upstream/qmp-unstable/target-s390x/arch_dump.c:17:0: /home/lcapitulino/work/src/upstream/qmp-unstable/include/sysemu/dump.h:24:5: note: previous declaration of ‘cpu_get_dump_info’ was here int cpu_get_dump_info(ArchDumpInfo *info, ^ make[1]: *** [target-s390x/arch_dump.o] Error 1 make: *** [subdir-s390x-softmmu] Error 2 make: *** Waiting for unfinished jobs
Re: [Qemu-devel] [PATCH v2 5/9] block: vhdx - break endian translation functions out
On Thu, Aug 01, 2013 at 05:03:39PM +0200, Stefan Hajnoczi wrote: On Wed, Jul 31, 2013 at 11:23:50PM -0400, Jeff Cody wrote: diff --git a/block/vhdx.h b/block/vhdx.h index 2db6615..5e0a1d3 100644 --- a/block/vhdx.h +++ b/block/vhdx.h @@ -398,4 +398,17 @@ static inline void cpu_to_leguids(MSGUID *guid) cpu_to_le16s(guid-data3); } +void vhdx_header_le_import(VHDXHeader *h); +void vhdx_header_le_export(VHDXHeader *orig_h, VHDXHeader *new_h); +void vhdx_log_desc_le_import(VHDXLogDescriptor *d); +void vhdx_log_desc_le_export(VHDXLogDescriptor *d); +void vhdx_log_data_le_export(VHDXLogDataSector *d); +void vhdx_log_entry_hdr_le_import(VHDXLogEntryHeader *hdr); +void vhdx_log_entry_hdr_le_export(VHDXLogEntryHeader *hdr); + + + + + + #endif You are a generous man when it comes to newlines! Thanks! -Jeff
Re: [Qemu-devel] [PATCH v2 6/9] block: vhdx - update log guid in header, and first write tracker
On Wed, Jul 31, 2013 at 11:23:51PM -0400, Jeff Cody wrote:
@@ -998,6 +1006,16 @@ exit:
+/* Per the spec, on the first write of guest-visible data to the file the
+ * data write guid must be updated in the header */
+void vhdx_user_visible_write(BlockDriverState *bs, BDRVVHDXState *s)
+{
+if (s-first_visible_write) {
+s-first_visible_write = false;
+vhdx_update_headers(bs, s, true, NULL);
Error handling is missing.
[Qemu-devel] [PATCH 2/8] [PATCH RFC v3] s390-qemu: cpu hotplug - SCLP CPU Info
From: Jason J. Herne jjhe...@us.ibm.com
Implement the CPU data in SCLP Read SCP Info. And implement Read CPU Info
SCLP command. This data will be used by the guest to get information about hot
plugged cpus.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/sclp.c | 51 +++
include/hw/s390x/sclp.h | 32 +
2 files changed, 83 insertions(+)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index cb53d7e..da8cf7a 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -15,6 +15,7 @@
#include cpu.h
#include sysemu/kvm.h
#include exec/memory.h
+#include sysemu/sysemu.h
#include hw/s390x/sclp.h
@@ -31,7 +32,26 @@ static inline S390SCLPDevice *get_event_facility(void)
static void read_SCP_info(SCCB *sccb)
{
ReadInfo *read_info = (ReadInfo *) sccb;
+CPUState *cpu;
int shift = 0;
+int cpu_count = 0;
+int i = 0;
+
+for (cpu = first_cpu; cpu != NULL; cpu = cpu-next_cpu) {
+cpu_count++;
+}
+
+/* CPU information */
+read_info-entries_cpu = cpu_to_be16(cpu_count);
+read_info-offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
+read_info-highest_cpu = cpu_to_be16(max_cpus);
+
+for (i = 0; i cpu_count; i++) {
+read_info-entries[i].address = i;
+read_info-entries[i].type = 0;
+}
+
+read_info-facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
while ((ram_size (20 + shift)) 65535) {
shift++;
@@ -41,6 +61,34 @@ static void read_SCP_info(SCCB *sccb)
sccb-h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
}
+/* Provide information about the CPU */
+static void sclp_read_cpu_info(SCCB *sccb)
+{
+ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
+CPUState *cpu;
+int cpu_count = 0;
+int i = 0;
+
+for (cpu = first_cpu; cpu != NULL; cpu = cpu-next_cpu) {
+cpu_count++;
+}
+
+cpu_info-nr_configured = cpu_to_be16(cpu_count);
+cpu_info-offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
+cpu_info-nr_standby = cpu_to_be16(0);
+
+/* The standby offset is 16-byte for each CPU */
+cpu_info-offset_standby = cpu_to_be16(cpu_info-offset_configured
++ cpu_info-nr_configured*sizeof(CpuEntry));
+
+for (i = 0; i cpu_count; i++) {
+cpu_info-entries[i].address = i;
+cpu_info-entries[i].type = 0;
+}
+
+sccb-h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
+}
+
static void sclp_execute(SCCB *sccb, uint64_t code)
{
S390SCLPDevice *sdev = get_event_facility();
@@ -50,6 +98,9 @@ static void sclp_execute(SCCB *sccb, uint64_t code)
case SCLP_CMDW_READ_SCP_INFO_FORCED:
read_SCP_info(sccb);
break;
+case SCLP_CMDW_READ_CPU_INFO:
+sclp_read_cpu_info(sccb);
+break;
default:
sdev-sclp_command_handler(sdev-ef, sccb, code);
break;
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 174097d..89ae7d1 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -79,12 +79,44 @@ typedef struct SCCBHeader {
#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
+/* CPU information */
+typedef struct CpuEntry {
+uint8_t address;
+uint8_t reserved0[13];
+uint8_t type;
+uint8_t reserved1;
+} QEMU_PACKED CpuEntry;
+
typedef struct ReadInfo {
SCCBHeader h;
uint16_t rnmax;
uint8_t rnsize;
+uint8_t _reserved1[16 - 11]; /* 11-15 */
+uint16_t entries_cpu; /* 16-17 */
+uint16_t offset_cpu;/* 18-19 */
+uint8_t _reserved2[24 - 20]; /* 20-23 */
+uint8_t loadparm[8]; /* 24-31 */
+uint8_t _reserved3[48 - 32]; /* 32-47 */
+uint64_t facilities;/* 48-55 */
+uint8_t _reserved0[100 - 56];
+uint32_t rnsize2;
+uint64_t rnmax2;
+uint8_t _reserved4[120-112]; /* 112-119 */
+uint16_t highest_cpu;
+uint8_t _reserved5[128 - 122]; /* 122-127 */
+struct CpuEntry entries[0];
} QEMU_PACKED ReadInfo;
+typedef struct ReadCpuInfo {
+SCCBHeader h;
+uint16_t nr_configured; /* 8-9 */
+uint16_t offset_configured; /* 10-11 */
+uint16_t nr_standby;/* 12-13 */
+uint16_t offset_standby;/* 14-15 */
+uint8_t reserved0[24-16]; /* 16-23 */
+struct CpuEntry entries[0];
+} QEMU_PACKED ReadCpuInfo;
+
typedef struct SCCB {
SCCBHeader h;
char data[SCCB_DATA_LEN];
--
1.7.10.4
Re: [Qemu-devel] [PATCH v2 5/9] block: vhdx - break endian translation functions out
On Wed, Jul 31, 2013 at 11:23:50PM -0400, Jeff Cody wrote: diff --git a/block/vhdx.h b/block/vhdx.h index 2db6615..5e0a1d3 100644 --- a/block/vhdx.h +++ b/block/vhdx.h @@ -398,4 +398,17 @@ static inline void cpu_to_leguids(MSGUID *guid) cpu_to_le16s(guid-data3); } +void vhdx_header_le_import(VHDXHeader *h); +void vhdx_header_le_export(VHDXHeader *orig_h, VHDXHeader *new_h); +void vhdx_log_desc_le_import(VHDXLogDescriptor *d); +void vhdx_log_desc_le_export(VHDXLogDescriptor *d); +void vhdx_log_data_le_export(VHDXLogDataSector *d); +void vhdx_log_entry_hdr_le_import(VHDXLogEntryHeader *hdr); +void vhdx_log_entry_hdr_le_export(VHDXLogEntryHeader *hdr); + + + + + + #endif You are a generous man when it comes to newlines!
Re: [Qemu-devel] [PATCH v2 4/9] block: vhdx - log support struct and defines
On Thu, Aug 01, 2013 at 05:00:05PM +0200, Stefan Hajnoczi wrote:
On Wed, Jul 31, 2013 at 11:23:49PM -0400, Jeff Cody wrote:
@@ -318,6 +323,18 @@ typedef struct VHDXMetadataEntries {
uint16_t present;
} VHDXMetadataEntries;
+typedef struct VHDXLogEntries {
+uint64_t offset;
+uint64_t length;
+uint32_t head;
+uint32_t tail;
+} VHDXLogEntries;
+
+typedef struct VHDXLogEntryInfo {
+uint64_t sector_start;
+uint32_t desc_count;
+} VHDXLogEntryInfo;
An array of VHDXLogEntryInfo structs would be affected by alignment on
x86_64. a[1] != (void*)a + sizeof(VHDXLogEntryInfo).
IMO all on-disk structs should be QEMU_PACKED for safety.
Neither of the two structs above reflect on-disk structs (actually
VHDXLogEntryInfo is currently unused, so I should drop it from this
patch).
I should mention this in the commit message, since the patch context
isn't enough to show it, but the structs introduced in this patch are
below a comment demarcation line that calls out an end to the on-disk
structs:
/* - END VHDX SPECIFICATION STRUCTURES */
[Qemu-devel] [PATCH 7/8] [PATCH RFC v3] s390-qemu: cpu hotplug - Implement hot_add_cpu hook
From: Jason J. Herne jjhe...@us.ibm.com
Implement hot_add_cpu for S390 to allow hot plugging of cpus.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/s390-virtio-ccw.c |3 +++
target-s390x/cpu.c | 32
target-s390x/cpu.h |2 ++
3 files changed, 37 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index b469960..30b6a48 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -117,6 +117,9 @@ static QEMUMachine ccw_machine = {
.alias = s390-ccw,
.desc = VirtIO-ccw based S390 machine,
.init = ccw_init,
+#if !defined(CONFIG_USER_ONLY)
+.hot_add_cpu = ccw_hot_add_cpu,
+#endif
.block_default_type = IF_VIRTIO,
.no_cdrom = 1,
.no_floppy = 1,
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index c90a91c..60029d9 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -27,6 +27,8 @@
#include qemu-common.h
#include qemu/timer.h
#include hw/hw.h
+#include hw/s390x/sclp.h
+#include sysemu/sysemu.h
#ifndef CONFIG_USER_ONLY
#include sysemu/arch_init.h
#endif
@@ -154,6 +156,36 @@ static void s390_cpu_finalize(Object *obj)
#endif
}
+#if !defined(CONFIG_USER_ONLY)
+void ccw_hot_add_cpu(const int64_t id, Error **errp)
+{
+S390CPU *new_cpu;
+CPUState *cpu;
+const char *model_str;
+int cpu_count = 0;
+
+for (cpu = first_cpu; cpu != NULL; cpu = cpu-next_cpu) {
+cpu_count++;
+}
+
+if (cpu_count == max_cpus) {
+error_setg(errp, Maximum number of cpus already defined);
+return;
+}
+
+if (id != cpu_count) {
+error_setg(errp, Unable to add CPU: % PRIi64
+ , The next available id is %d, id, cpu_count);
+return;
+}
+
+model_str = s390_cpu_addr2state(0)-env.cpu_model_str;
+new_cpu = s390_new_cpu(model_str, id);
+object_property_set_bool(OBJECT(new_cpu), true, realized, NULL);
+raise_irq_cpu_hotplug();
+}
+#endif
+
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 0f68dd0..711aad4 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -383,6 +383,8 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
void s390_add_running_cpu(S390CPU *cpu);
unsigned s390_del_running_cpu(S390CPU *cpu);
+void ccw_hot_add_cpu(const int64_t id, Error **errp);
+
/* service interrupts are floating therefore we must not pass an cpustate */
void s390_sclp_extint(uint32_t parm);
--
1.7.10.4
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On 08/01/13 15:08, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Only happens when also changing the machine type on upgrade as it is turned off on old machine types. But, yes, pvpanic will show up as Unknown device without driver and with the funky yellow exclamation mark in device manager in windows guests. Newer windows versions don't kick the new device wizard. But still I have my doubts that it is a good idea to add it unconditionally ... cheers, Gerd
Re: [Qemu-devel] qemu virtfs 9p not working on arm?
On Thu, Aug 01, 2013 at 10:43:01AM +0100, Peter Maydell wrote: On 1 August 2013 00:25, Rich Felker dal...@aerifal.cx wrote: I'm not sure whether this is a kernel problem or a qemu problem (or even a user problem). The folks on OFTC #qemu suggested I email the list and relevant section maintainers so that's what I'm doing -- hope it's okay. I'm trying to boot qemu-system-arm with rootfs on 9p, and experiencing the following error: You don't say which board model you're using; I guess one of the PCI ones from the rest of the command line. I'm using -M versatilepb -cpu arm1136-r2, also copied from the Aboriginal Linux image since that's what the kernel was intended to run on. I'm on the software side, not the hardware side, so I'm far from an expert in understanding arm hardware variants. (I'm using qemu to test software and make sure the arm-specific asm is correct, not to test deployments.) From what I understand, this board has PCI. However, while doing some searches based on your question, I found this which may be relevant: http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg02237.html Does this sound like it could be the issue? | 9pnet: Installing 9P2000 support | 9pnet_virtio: probe of virtio0 failed with error -2 | VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 4 | rtc-pl031 dev:e8: setting system clock to 2013-07-31 22:56:25 UTC (1375311385) | 9pnet_virtio: no channels available | VFS: Cannot open root device root or unknown-block(0,0): error -2 | Please append a correct root= boot option; here are the available partitions: | 0b00 1048575 sr0 driver: sr | Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) That looks like ENOENT during the probe to me, but I can't find where in the kernel source it's coming from. My configuration is: -append root=root rw rootflags=rw,trans=virtio,version=9p2000.L -rootfstype=9p -fsdev local,id=root,path=$(pwd)/root,security_model=none -device virtio-9p-pci,fsdev=root,mount_tag=root and this same configuration seems to work on x86. The kernels I'm using are from the latest Aboriginal Linux images (3.10) and /proc/config.gz seems to show that all the 9p/virtfs options are enabled. Do other virtio devices work? I haven't really tried any devices since my aim is just software testing, but I could try. Rich
Re: [Qemu-devel] [PATCH 8/8] [PATCH RFC v3] qemu-monitor: HMP cpu-add wrapper
Luiz,
Am 01.08.2013 16:12, schrieb Jason J. Herne:
From: Jason J. Herne jjhe...@us.ibm.com
Add HMP cpu-add wrapper to allow cpu hot plugging via monitor.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
What are your thoughts on this?
Thanks,
Andreas
---
hmp-commands.hx | 13 +
hmp.c | 10 ++
hmp.h |1 +
3 files changed, 24 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8c6b91a..cb8712b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1587,6 +1587,19 @@ Executes a qemu-io command on the given block device.
ETEXI
{
+.name = cpu-add,
+.args_type = id:i,
+.params = id,
+.help = add cpu,
+.mhandler.cmd = hmp_cpu_add,
+},
+
+STEXI
+@item cpu-add @var{id}
+Add CPU with id @var{id}
+ETEXI
+
+{
.name = info,
.args_type = item:s?,
.params = [subcommand],
diff --git a/hmp.c b/hmp.c
index c45514b..9465bd4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1475,6 +1475,16 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict
*qdict)
hmp_handle_error(mon, errp);
}
+void hmp_cpu_add(Monitor *mon, const QDict *qdict)
+{
+int cpuid;
+Error *err = NULL;
+
+cpuid = qdict_get_int(qdict, id);
+qmp_cpu_add(cpuid, err);
+hmp_handle_error(mon, err);
+}
+
void hmp_chardev_add(Monitor *mon, const QDict *qdict)
{
const char *args = qdict_get_str(qdict, args);
diff --git a/hmp.h b/hmp.h
index 6c3bdcd..9effca5 100644
--- a/hmp.h
+++ b/hmp.h
@@ -87,5 +87,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_cpu_add(Monitor *mon, const QDict *qdict);
#endif
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On 08/01/2013 08:18 AM, Gerd Hoffmann wrote: On 08/01/13 15:08, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Only happens when also changing the machine type on upgrade as it is turned off on old machine types. But, yes, pvpanic will show up as Unknown device without driver and with the funky yellow exclamation mark in device manager in windows guests. Newer windows versions don't kick the new device wizard. But still I have my doubts that it is a good idea to add it unconditionally ... Automatic devices with no command line argument have proven to be a nightmare for libvirt as well. Although the just-released libvirt 1.1.1 now supports the on_crash element for controlling the command line parameters of qemu related to how qemu will behave when the pvpanic device is triggered, I would also welcome having the ability to control whether the guest even has a pvpanic device exposed, just as we can control whether a guest has a memballoon device exposed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On Thu, 2013-08-01 at 19:31 +0300, Michael S. Tsirkin wrote: On Thu, Aug 01, 2013 at 10:26:53AM -0600, Eric Blake wrote: On 08/01/2013 08:18 AM, Gerd Hoffmann wrote: On 08/01/13 15:08, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Only happens when also changing the machine type on upgrade as it is turned off on old machine types. But, yes, pvpanic will show up as Unknown device without driver and with the funky yellow exclamation mark in device manager in windows guests. Newer windows versions don't kick the new device wizard. But still I have my doubts that it is a good idea to add it unconditionally ... Automatic devices with no command line argument have proven to be a nightmare for libvirt as well. Although the just-released libvirt 1.1.1 now supports the on_crash element for controlling the command line parameters of qemu related to how qemu will behave when the pvpanic device is triggered, I would also welcome having the ability to control whether the guest even has a pvpanic device exposed, just as we can control whether a guest has a memballoon device exposed. A natural way to do this would be with -device pvpanic. I'm not sure why it wasn't done like this from the beginning, but it shouldn't be hard to redo, hopefully we can fix this bug in time for 1.6. I'll come up with something, hopefully in time. Marcel
Re: [Qemu-devel] qemu virtfs 9p not working on arm?
On 1 August 2013 16:59, Rich Felker dal...@aerifal.cx wrote: I'm using -M versatilepb -cpu arm1136-r2, also copied from the Aboriginal Linux image since that's what the kernel was intended to run on. I'm on the software side, not the hardware side, so I'm far from an expert in understanding arm hardware variants. (I'm using qemu to test software and make sure the arm-specific asm is correct, not to test deployments.) From what I understand, this board has PCI. However, while doing some searches based on your question, I found this which may be relevant: http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg02237.html Does this sound like it could be the issue? It might be. If so, you'll probably find that other PCI devices (including but not limited to other virtio devices) also don't work -- this will be useful in narrowing down whether the problem is specific to our virtio-9p code somehow (seems unlikely as it doesn't do anything particularly odd for a virtio device), or specific to virtio-pci (unlikely again, there's no ARM specific code at that point) or related to PCI. The kernel's support for the PCI controller on this board is definitely known to be pretty buggy and broken, so it's a strong candidate for the culprit here. -- PMM
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On Thu, Aug 01, 2013 at 10:26:53AM -0600, Eric Blake wrote: On 08/01/2013 08:18 AM, Gerd Hoffmann wrote: On 08/01/13 15:08, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Only happens when also changing the machine type on upgrade as it is turned off on old machine types. But, yes, pvpanic will show up as Unknown device without driver and with the funky yellow exclamation mark in device manager in windows guests. Newer windows versions don't kick the new device wizard. But still I have my doubts that it is a good idea to add it unconditionally ... Automatic devices with no command line argument have proven to be a nightmare for libvirt as well. Although the just-released libvirt 1.1.1 now supports the on_crash element for controlling the command line parameters of qemu related to how qemu will behave when the pvpanic device is triggered, I would also welcome having the ability to control whether the guest even has a pvpanic device exposed, just as we can control whether a guest has a memballoon device exposed. A natural way to do this would be with -device pvpanic. I'm not sure why it wasn't done like this from the beginning, but it shouldn't be hard to redo, hopefully we can fix this bug in time for 1.6. -- MST
[Qemu-devel] net/tap.c: Possibly a way to stall tap input
Hi all,
I'm tracking down a nasty stall of tap input over a custom 1.3.x QEMU
version. Under certain load, our tap backend stops reading from the char
device, and that even if we reset the guest. The frontend device
(pcnet32) is able to receive (can_receive would return 0), but the
tap's fd is no longer registered with the iohandler list.
I was digging into the involved code and found something fishy:
net/tap.c:
static void tap_send(void *opaque)
{
...
size = qemu_send_packet_async(s-nc, buf, size,
tap_send_completed);
if (size == 0) {
tap_read_poll(s, false);
}
So, if tap_send is registered for the mainloop polling (ie. can_receive
returned true before starting to poll) but qemu_send_packet_async
returns 0 now as qemu_can_send_packet/can_receive happens to report
false in the meantime, we will disable read polling. If also write
polling is off, the fd will be completely removed from the iohandler
list. But even if write polling remains on, I wonder what should bring
read polling back?
We only have an unhandy reproduction scenario, so I wasn't able to
confirm this theory on the target yet (and will not be before Monday,
unfortunately). But any comments on this would be very welcome.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 8/8] [PATCH RFC v3] qemu-monitor: HMP cpu-add wrapper
On Thu, 01 Aug 2013 18:02:08 +0200
Andreas Färber afaer...@suse.de wrote:
Luiz,
Am 01.08.2013 16:12, schrieb Jason J. Herne:
From: Jason J. Herne jjhe...@us.ibm.com
Add HMP cpu-add wrapper to allow cpu hot plugging via monitor.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
What are your thoughts on this?
Looks good:
Reviewed-by: Luiz Capitulino lcapitul...@redhat.com
Thanks,
Andreas
---
hmp-commands.hx | 13 +
hmp.c | 10 ++
hmp.h |1 +
3 files changed, 24 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8c6b91a..cb8712b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1587,6 +1587,19 @@ Executes a qemu-io command on the given block device.
ETEXI
{
+.name = cpu-add,
+.args_type = id:i,
+.params = id,
+.help = add cpu,
+.mhandler.cmd = hmp_cpu_add,
+},
+
+STEXI
+@item cpu-add @var{id}
+Add CPU with id @var{id}
+ETEXI
+
+{
.name = info,
.args_type = item:s?,
.params = [subcommand],
diff --git a/hmp.c b/hmp.c
index c45514b..9465bd4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1475,6 +1475,16 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict
*qdict)
hmp_handle_error(mon, errp);
}
+void hmp_cpu_add(Monitor *mon, const QDict *qdict)
+{
+int cpuid;
+Error *err = NULL;
+
+cpuid = qdict_get_int(qdict, id);
+qmp_cpu_add(cpuid, err);
+hmp_handle_error(mon, err);
+}
+
void hmp_chardev_add(Monitor *mon, const QDict *qdict)
{
const char *args = qdict_get_str(qdict, args);
diff --git a/hmp.h b/hmp.h
index 6c3bdcd..9effca5 100644
--- a/hmp.h
+++ b/hmp.h
@@ -87,5 +87,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict
*qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_cpu_add(Monitor *mon, const QDict *qdict);
#endif
Re: [Qemu-devel] net/tap.c: Possibly a way to stall tap input
On 2013-08-01 19:15, Jan Kiszka wrote:
Hi all,
I'm tracking down a nasty stall of tap input over a custom 1.3.x QEMU
version. Under certain load, our tap backend stops reading from the char
device, and that even if we reset the guest. The frontend device
(pcnet32) is able to receive (can_receive would return 0), but the
^^^
Yes, the pcnet lacks qemu_flush_queued_packets, like certain other NIC
models already have. We added that to pcnet_init and pcnet_start (patch
will follow), but that didn't make a difference, likely due to what I
described below.
Jan
tap's fd is no longer registered with the iohandler list.
I was digging into the involved code and found something fishy:
net/tap.c:
static void tap_send(void *opaque)
{
...
size = qemu_send_packet_async(s-nc, buf, size,
tap_send_completed);
if (size == 0) {
tap_read_poll(s, false);
}
So, if tap_send is registered for the mainloop polling (ie. can_receive
returned true before starting to poll) but qemu_send_packet_async
returns 0 now as qemu_can_send_packet/can_receive happens to report
false in the meantime, we will disable read polling. If also write
polling is off, the fd will be completely removed from the iohandler
list. But even if write polling remains on, I wonder what should bring
read polling back?
We only have an unhandy reproduction scenario, so I wasn't able to
confirm this theory on the target yet (and will not be before Monday,
unfortunately). But any comments on this would be very welcome.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] xhci: implement warm port reset
Without this patch windows can't do port resets for usb3 devices.
https://bugzilla.redhat.com/show_bug.cgi?id=949514
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/hcd-xhci.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index a922cb4..c783a11 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2686,7 +2686,7 @@ static void xhci_port_update(XHCIPort *port, int
is_detach)
xhci_port_notify(port, PORTSC_CSC);
}
-static void xhci_port_reset(XHCIPort *port)
+static void xhci_port_reset(XHCIPort *port, bool warm_reset)
{
trace_usb_xhci_port_reset(port-portnr);
@@ -2697,6 +2697,11 @@ static void xhci_port_reset(XHCIPort *port)
usb_device_reset(port-uport-dev);
switch (port-uport-dev-speed) {
+case USB_SPEED_SUPER:
+if (warm_reset) {
+port-portsc |= PORTSC_WRC;
+}
+/* fall through */
case USB_SPEED_LOW:
case USB_SPEED_FULL:
case USB_SPEED_HIGH:
@@ -2859,8 +2864,12 @@ static void xhci_port_write(void *ptr, hwaddr reg,
switch (reg) {
case 0x00: /* PORTSC */
/* write-1-to-start bits */
+if (val PORTSC_WPR) {
+xhci_port_reset(port, true);
+break;
+}
if (val PORTSC_PR) {
-xhci_port_reset(port);
+xhci_port_reset(port, false);
break;
}
--
1.7.9.7
Re: [Qemu-devel] [PATCH v2 2/9] block: vhdx - add header update capability.
On Wed, Jul 31, 2013 at 11:23:47PM -0400, Jeff Cody wrote:
@@ -212,6 +242,24 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size,
int crc_offset)
/*
+ * This generates a UUID that is compliant with the MS GUIDs used
+ * in the VHDX spec (and elsewhere).
+ *
+ * We can do this with uuid_generate if uuid.h is present,
+ * however not all systems have uuid and the generation is
+ * pretty straightforward for the DCE + random usage case
The talk of uuid.h not being available confuses me. The code has no
#ifdef and we do not define uuid_generate() ourselves.
Is this an outdated comment?
+/* Update the VHDX headers
+ *
+ * This follows the VHDX spec procedures for header updates.
+ *
+ * - non-current header is updated with largest sequence number
+ */
+static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s, bool
rw)
rw should be named generate_file_write_guid. If you call
vhdx_update_header() again later you do not need to update FileWriteGuid
again according to the spec. There's probably no harm in doing so but
the spec explicitly says If this is the first header update within the
session, use a new value for the FileWriteGuid.
This means that future calls to vhdx_update_header() in the same session
should set generate_file_write_guid to false. Therefore rw is not
the right name.
+{
+int ret = 0;
+int hdr_idx = 0;
+uint64_t header_offset = VHDX_HEADER1_OFFSET;
+
+VHDXHeader *active_header;
+VHDXHeader *inactive_header;
+VHDXHeader header_le;
+uint8_t *buffer;
+
+/* operate on the non-current header */
+if (s-curr_header == 0) {
+hdr_idx = 1;
+header_offset = VHDX_HEADER2_OFFSET;
+}
+
+active_header = s-headers[s-curr_header];
+inactive_header = s-headers[hdr_idx];
+
+inactive_header-sequence_number = active_header-sequence_number + 1;
+
+/* a new file guid must be generate before any file write, including
s/generate/generated/
+ * headers */
+memcpy(inactive_header-file_write_guid, s-session_guid,
+ sizeof(MSGUID));
+
+/* a new data guid only needs to be generate before any guest-visible
+ * writes, so update it if the image is opened r/w. */
+if (rw) {
+vhdx_guid_generate(inactive_header-data_write_guid);
+}
+
+/* the header checksum is not over just the packed size of VHDXHeader,
+ * but rather over the entire 'reserved' range for the header, which is
+ * 4KB (VHDX_HEADER_SIZE). */
+
+buffer = qemu_blockalign(bs, VHDX_HEADER_SIZE);
+/* we can't assume the extra reserved bytes are 0 */
+ret = bdrv_pread(bs-file, header_offset, buffer, VHDX_HEADER_SIZE);
+if (ret 0) {
+goto exit;
+}
+/* overwrite the actual VHDXHeader portion */
+memcpy(buffer, inactive_header, sizeof(VHDXHeader));
+inactive_header-checksum = vhdx_update_checksum(buffer,
+ VHDX_HEADER_SIZE, 4);
+vhdx_header_le_export(inactive_header, header_le);
+bdrv_pwrite_sync(bs-file, header_offset, header_le,
sizeof(VHDXHeader));
This function can fail and it's a good indicator that future I/Os will
also be in trouble. Please propagate the error return.
@@ -801,8 +955,7 @@ static int vhdx_open(BlockDriverState *bs, QDict
*options, int flags)
}
if (flags BDRV_O_RDWR) {
-ret = -ENOTSUP;
-goto fail;
+vhdx_update_headers(bs, s, false);
Error handling is missing. At this point it looks like we cannot write
to the file. Propagating the error seems reasonable.
[Qemu-devel] [PATCH 0/8] [PATCH RFC v3] s390 cpu hotplug
From: Jason J. Herne jjhe...@us.ibm.com Latest code for cpu Hotplug on S390 architecture. This one is vastly simpler than v2 as we have decided to avoid the command line specification of -device s390-cpu. The last version can be found here: http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg01183.html There is also a patch in this series to add cpu-add to the Qemu monitor interface. Hotplugged cpus are created in the configured state and can be used by the guest after the guest onlines the cpu by: echo 1 /sys/bus/cpu/devices/cpuN/online Hot unplugging is currently not implemented by this code. Jason J. Herne (8): s390-qemu: cpu hotplug - Define New SCLP Codes s390-qemu: cpu hotplug - SCLP CPU Info s390-qemu: cpu hotplug - SCLP Event integration s390-qemu: cpu hotplug - Storage key global access s390-qemu: cpu hotplug - ipi_states enhancements s390-qemu: cpu hotplug - s390 cpu init improvements for hotplug s390-qemu: cpu hotplug - Implement hot_add_cpu hook qemu-monitor: HMP cpu-add wrapper hmp-commands.hx | 13 hmp.c | 10 hmp.h |1 + hw/s390x/Makefile.objs|2 +- hw/s390x/event-facility.c |7 +++ hw/s390x/s390-virtio-ccw.c|8 ++- hw/s390x/s390-virtio.c| 47 +-- hw/s390x/s390-virtio.h|2 +- hw/s390x/sclp.c | 53 +++- hw/s390x/sclpcpu.c| 120 + include/hw/s390x/event-facility.h |3 + include/hw/s390x/sclp.h | 41 + target-s390x/cpu.c| 36 ++- target-s390x/cpu.h|7 +++ target-s390x/helper.c | 12 15 files changed, 336 insertions(+), 26 deletions(-) create mode 100644 hw/s390x/sclpcpu.c -- 1.7.10.4
[Qemu-devel] default slot used for vga device on q35 machines
libvirt makes an assumption that if you specify -vga qxl instead of -device qxl-vga,..., the vga device will be connected to slot 2. I learned this in a recent discussion about a bug caused by switching over to using the former syntax (in order to support multiheaded QXL): https://bugzilla.redhat.com/show_bug.cgi?id=981094#c9 Since then, while working on proper support for the q35 machine type in libvirt, I did a test run of: qemu-kvm -M q35 -nodefaults -nodefconfig -qmp unix:/tmp/qemu,server -vnc :15 -vga std -usb Then ran query-pci in the qmp monitor and found that the vga device is put at slot 1 instead of slot 2. My questions: 1) Is this difference intentional, or a bug? 2) If it's intentional, will the device always be at slot 1 (and trigger an error if something else is also placed at slot 1), or is it just picking the first unused slot? (that would *not* be good, because we must be able to predict what device is in which slot and prevent them from changing from run to run). 3) Does the qxl multihead support really require that the device be at slot 2 (as stated in the above bugzilla commend)? Or is that just a misunderstanding/overstatement?
Re: [Qemu-devel] [PATCH v5 0/2] e1000: add interrupt mitigation support
Ok, so back to the one-patch version! :) I'll prepare it asap.
Thanks,
Vincenzo
2013/8/1 Andreas Färber afaer...@suse.de:
Am 01.08.2013 11:38, schrieb Stefan Hajnoczi:
On Wed, Jul 31, 2013 at 03:39:05PM +0200, Vincenzo Maffione wrote:
Ok, but it's unclear how do you prefer to create and empty
PC_COMPAT_1_6 in Patch 1.
If you want to keep this declaration form
[...]
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_6,
{ /* end of list */ }
},
[...]
in the two pc_*_machine_v1_6 structs, I'm forced to define
#define PC_COMPAT_1_6 { /*empty*/ }
but then I can't extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as header
(like you guys do for PC_COMPAT_1_5 and PC_COMPAT_1_4), because
otherwise PC_COMPAT_1_6 would act as a premature terminator for
PC_COMPAT_1_5 (right?).
Should I extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as a tail, or
should I avoid extending it in the Patch 1, and do the extension in
Patch 2 (when I have a non-empty PC_COMPAT_1_6)?
You are right, (GlobalProperty[]) {, {...}} is not valid syntax. In
that case I would switch PC_COMPAT_1_6 into the e1000 interrupt
mitigation patch. That way the patches are bisectable.
You can still introduce the QEMU 1.7 pc machine type as a separate
patch if you wish, but I no longer see a big win if PC_COMPAT_1_6 cannot
be isolated from the e1000 change.
Andreas: Do you agree to do everything in a single patch?
I see now that it wouldn't work with an empty macro (unless we drop the
,{} and then later have to remember to add it back, which may be even
worse; my main concern was having the property set in the actual patch
for bisecting and cherry-picking, so no objections from my side.
mst was the other candidate for needing compat_props for now-delayed ACPI.
Stefan, you haven't replied wrt VMXNET3 and MSI-X yet - that may be
another candidate if we can't break migration format as proposed.
But we can always introduce the same machine in several patches, we just
need to be careful with merging them to not get multiple 1.7 machines
and not to loose properties.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
Vincenzo Maffione
Re: [Qemu-devel] [PATCH] target-mips: fix 34Kf configuration for DSP ASE
Hi Yongbok, You need to make Status.MX writeable as well. - .CP0_Status_rw_bitmask = 0x3678FF1F, + .CP0_Status_rw_bitmask = 0x3778FF1F, -Eric -Original Message- From: qemu-devel-bounces+eric.johnson=imgtec@nongnu.org [mailto:qemu-devel- bounces+eric.johnson=imgtec@nongnu.org] On Behalf Of Yongbok Kim Sent: Thursday, August 01, 2013 3:35 AM To: qemu-devel@nongnu.org Cc: Yongbok Kim; Cristian Cuna; Leon Alrae; aurel...@aurel32.net Subject: [Qemu-devel] [PATCH] target-mips: fix 34Kf configuration for DSP ASE 34Kf core does support DSP ASE. CP0_Config3 configuration for 34Kf and description are wrong. Please refer to MIPS32(R) 34Kf(TM) Processor Core Datasheet Signed-off-by: Yongbok Kim yongbok@imgtec.com --- target-mips/translate_init.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index 7cf238f..4cd9ed5 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -274,14 +274,13 @@ static const mips_def_t mips_defs[] = (0 CP0C1_DS) | (3 CP0C1_DL) | (1 CP0C1_DA) | (1 CP0C1_CA), .CP0_Config2 = MIPS_CONFIG2, -.CP0_Config3 = MIPS_CONFIG3 | (1 CP0C3_VInt) | (1 CP0C3_MT), +.CP0_Config3 = MIPS_CONFIG3 | (1 CP0C3_VInt) | (1 CP0C3_MT) | + (1 CP0C3_DSPP), .CP0_LLAddr_rw_bitmask = 0, .CP0_LLAddr_shift = 0, .SYNCI_Step = 32, .CCRes = 2, -/* No DSP implemented. */ .CP0_Status_rw_bitmask = 0x3678FF1F, -/* No DSP implemented. */ .CP0_TCStatus_rw_bitmask = (0 CP0TCSt_TCU3) | (0 CP0TCSt_TCU2) | (1 CP0TCSt_TCU1) | (1 CP0TCSt_TCU0) | (0 CP0TCSt_TMX) | (1 CP0TCSt_DT) | -- 1.7.4
[Qemu-devel] [PATCH 5/8] [PATCH RFC v3] s390-qemu: cpu hotplug - ipi_states enhancements
From: Jason J. Herne jjhe...@us.ibm.com
Modify s390_cpu_addr2state to allow fetching state information for cpu addresses
above smp_cpus. Hotplug requires this capability.
Also add s390_cpu_set_state function to allow modification of ipi_state entries
during hotplug.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/s390-virtio.c |9 +
target-s390x/cpu.h |2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 21e9124..5ad9cf3 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -54,12 +54,13 @@
static VirtIOS390Bus *s390_bus;
static S390CPU **ipi_states;
-S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
+void s390_cpu_set_ipistate(uint16_t cpu_addr, S390CPU *state)
{
-if (cpu_addr = smp_cpus) {
-return NULL;
-}
+ipi_states[cpu_addr] = state;
+}
+S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
+{
return ipi_states[cpu_addr];
}
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 877eac7..62eb810 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -377,7 +377,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU
*cpu, int type,
uint8_t *s390_get_storage_keys(void);
void s390_alloc_storage_keys(ram_addr_t ram_size);
-
+void s390_cpu_set_ipistate(uint16_t cpu_addr, S390CPU *state);
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
void s390_add_running_cpu(S390CPU *cpu);
unsigned s390_del_running_cpu(S390CPU *cpu);
--
1.7.10.4
[Qemu-devel] [ANNOUNCE] QEMU 1.6.0-rc1 is now available
Hi, On behalf of the QEMU Team, I'd like to announce the availability of the second release candidate for the QEMU 1.6 release. This release is meant for testing purposes and should not be used in a production environment. http://wiki.qemu.org/download/qemu-1.6.0-rc1.tar.bz2 You can help improve the quality of the QEMU 1.6 release by testing this release and reporting bugs on Launchpad: https://bugs.launchpad.net/qemu/ The release plan for the 1.6 release is available at: http://wiki.qemu.org/Planning/1.6 Please add entries to the ChangeLog for the 1.6 release below: http://wiki.qemu.org/ChangeLog/1.6 The following changes have been made since v1.6.0-rc0: - virtio-console: Use exitfn for virtserialport, too (Andreas Färber) - virtio-9p-device: Avoid freeing uninitialized memory (Andreas Färber) - migration: don't use uninitialized variables (Pawit Pornkitprasan) - migration: send total time in QMP at completed stage (Pawit Pornkitprasan) - MAINTAINERS: change Igor Mitsyanko's email address (Igor Mitsyanko) - qdev: Use clz in print_size (Richard Henderson) - qdev: Fix 32-bit compilation in print_size (Richard Henderson) - mips_r4k: Silence BIOS loading warning for qtest (Andreas Färber) - mips_jazz: Silence BIOS loading warning for qtest (Andreas Färber) - mips_malta: Silence BIOS loading warning for qtest (Andreas Färber) - mips_fulong2e: Silence BIOS loading warning for qtest (Andreas Färber) - target-ppc: Suppress TCG instruction emulation warnings for qtest (Andreas Färber) - chardev: fix CHR_EVENT_OPENED events for mux chardevs (Michael Roth) - tci: Fix broken build (compiler warning caused by redefined macro BIT) (Stefan Weil) - target-mips: correct the values in the DSP tests (Petar Jovanovic) - s390: Implement dump-guest-memory support for target s390x (Ekaterina Tumanova) - s390x/kvm: Remove redundant return code (Thomas Huth) - s390x/kvm: Reworked/fixed handling of cc3 in kvm_handle_css_inst() (Thomas Huth) - s390x/ioinst: Fixed priority of operand exceptions (Thomas Huth) - s390x/ioinst: Fixed alignment check in SCHM instruction (Thomas Huth) - s390x/ioinst: Throw addressing exception when memory_map failed (Thomas Huth) - s390x/ioinst: Add missing alignment checks for IO instructions (Thomas Huth) - s390/sclpconsole: handle char layer busy conditions (Heinz Graalfs) - hcd-ohci: add dma error handling (Alexey Kardashevskiy) - uhci: egsm fix (Gerd Hoffmann) - xhci: handle USB_RET_IOERROR (Gerd Hoffmann) - spice: fix display initialization (Gerd Hoffmann) Regards, Anthony Liguori
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On 08/01/2013 06:26 PM, Eric Blake wrote: On 08/01/2013 08:18 AM, Gerd Hoffmann wrote: On 08/01/13 15:08, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Only happens when also changing the machine type on upgrade as it is turned off on old machine types. But, yes, pvpanic will show up as Unknown device without driver and with the funky yellow exclamation mark in device manager in windows guests. Newer windows versions don't kick the new device wizard. But still I have my doubts that it is a good idea to add it unconditionally ... Automatic devices with no command line argument have proven to be a nightmare for libvirt as well. Although the just-released libvirt 1.1.1 now supports the on_crash element for controlling the command line parameters of qemu related to how qemu will behave when the pvpanic device is triggered, I would also welcome having the ability to control whether the guest even has a pvpanic device exposed, just as we can control whether a guest has a memballoon device exposed. This is quite different from memballoon. pvpanic is a single I/O port, it doesn't use up a PCI slot (thus causing conflicts with other devices at the same address). Perhaps this issue is simply fixed by making the _STA method return 0x0B instead of 0x0F (i.e. turning off the show in user interface bit). Paolo
Re: [Qemu-devel] [PATCH v3] semaphore: fix a hangup problem under load on NetBSD hosts.
On 08/01/2013 05:24 AM, Brad wrote: On 03/07/13 5:41 AM, Laszlo Ersek wrote: On 07/03/13 10:58, Izumi Tsutsui wrote: Fix following bugs in fallback implementation of counting semaphores with mutex+condvar added in c166cb72f1676855816340666c3b618beef4b976: - waiting threads are not restarted properly if more than one threads are waiting unblock signals in qemu_sem_timedwait() - possible missing pthread_cond_signal(3) calls when waiting threads are returned by ETIMEDOUT - fix an uninitialized variable The problem is analyzed by and fix is provided by Noriyuki Soda. Also put additional cleanup suggested by Laszlo Ersek: - make QemuSemaphore.count unsigned (it won't be negative) - check a return value of in pthread_cond_wait() in qemu_sem_wait() Signed-off-by: Izumi Tsutsui tsut...@ceres.dti.ne.jp Reviewed-by: Laszlo Ersek ler...@redhat.com --- v3: - fix a missed assignment and actually check a retval of pthread_cond_wait() Compared v3 against v2. Reviewed-by: Laszlo Ersek ler...@redhat.com Laszlo This patch seems to have been dropped. CCing Anthony and qemu-stable. Paolo
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On 08/01/2013 04:23 PM, Paolo Bonzini wrote: Automatic devices with no command line argument have proven to be a nightmare for libvirt as well. Although the just-released libvirt 1.1.1 now supports the on_crash element for controlling the command line parameters of qemu related to how qemu will behave when the pvpanic device is triggered, I would also welcome having the ability to control whether the guest even has a pvpanic device exposed, just as we can control whether a guest has a memballoon device exposed. This is quite different from memballoon. pvpanic is a single I/O port, it doesn't use up a PCI slot (thus causing conflicts with other devices at the same address). Perhaps this issue is simply fixed by making the _STA method return 0x0B instead of 0x0F (i.e. turning off the show in user interface bit). That may fix the issue of a windows guest showing the yellow ! mark, but what if, down the road, someone writes an actual windows driver that is aware of that port and how to make a windows BSOD write a panic notification to the port? How does a user go about installing such a driver if the device is not exposed in the user interface list of devices? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On Thu, Aug 01, 2013 at 04:08:57PM +0300, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Now what will happen on a cluster with hundreds of such VMs? If that cluster has a health monitoring service it may show all the VMs in a not healthy state. My point is that a device that requires a driver that is not inbox, should not be present by default. One possible solution is to add it manually with -device from command line. Any thoughts? Marcel Interesting. You are basically saying we should have a rule that no new builtin devices should be added without an explicit request from management interface? -- MST
Re: [Qemu-devel] [PATCH v2 2/2] kvm: migrate vPMU state
KVM disabled HW counters when outside of a guest mode (otherwise result
will be useless), so I do not see how the problem you describe can
happen.
Yes, you're right.
On the other hand MPU emulation assumes that counter have to be disabled
while MSR_IA32_PERFCTR0 is written since write to MSR_IA32_PERFCTR0 does
not reprogram perf evens, so we need either disable/enable counters to
write MSR_IA32_PERFCTR0 or have this patch in the kernel:
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 5c4f631..bf14e42 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -412,6 +412,8 @@ int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct
msr_data *msr_info)
if (!msr_info-host_initiated)
data = (s64)(s32)data;
pmc-counter += data - read_pmc(pmc);
+ if (msr_info-host_initiated)
+ reprogram_gp_counter(pmc, pmc-eventsel);
return 0;
} else if ((pmc = get_gp_pmc(pmu, index, MSR_P6_EVNTSEL0))) {
if (data == pmc-eventsel)
Why do you need if (msr_info-host_initiated)? I could not find any
hint in the manual that the overflow counter will still use the value
of the counter that was programmed first.
If we need to do it always, I agree it's better to modify the QEMU
patch and not disable/enable the counters. But if we need to restrict
it to host-initiated writes, I would rather have the QEMU patch as I
posted it. So far we always had less side-effects from host_initiated,
not more, and I think it's a good rule of thumb.
Paolo
Re: [Qemu-devel] [edk2] SetVirtualAddressMap and NX bit
+ Matt. On Wed, Jul 31, 2013 at 02:10:04PM +0200, Laszlo Ersek wrote: Just random ideas... First of all, thanks for looking. You made me look too and find the fun :-) The fact that you guys didn't say Oh yeah, we do this because... but simply shruggingly suggested ideas should've been enough to give me the hint to look in our own backyard and maybe to permit the possibility of the kernel doing something funny. And it does, indeed! And for that you need to look at SetVirtualAddressMap() itself or rather, how we call it: phys_efi_set_virtual_address_map |- efi_call_phys_prelog |- efi_call_phys4(efi_phys.set_virtual_address_map |- efi_call_phys_epilog Now guess what those pre- and epi- things do. Right: efi_call_phys_prelog does early_code_mapping_set_exec(1) and efi_call_phys_epilog does early_code_mapping_set_exec(0) and we end up with that PTE's NX bit set: before: [ 47.379000] __lookup_address_in_pgd: pte: 0x7fb12063 (0x88007c823b68) after: [ 47.393000] __lookup_address_in_pgd: pte: 0x80007fb12163 (0x88007c823b68) What is still missing from the big picture is why the PTE in my pagetable (not the kernel's pagetable) gets that bit set?? I mean, the EFI code is using pgd_offset_k() which looks at init_mm and my PGD is a different one. And I guess the explanation for that would also clarify why this doesn't happen on baremetal so probably it has something to do with the nested page table thingy. Oh well... -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --
Re: [Qemu-devel] [PATCH] cpus: use cpu_is_stopped efficiently
On Thu, Aug 01, 2013 at 04:12:03PM +0800, “tiejun.chen” wrote: On 07/26/2013 04:47 PM, Tiejun Chen wrote: It makes more sense and simple later. Any feedback :) Tiejun Reviewed-by: Marcelo Tosatti mtosa...@redhat.com
[Qemu-devel] [PULL for-1.6 0/2] usb fixes
Hi, Two more little usb fixes for 1.6. please pull, Gerd The following changes since commit 75e2a4baf1536682d111d9bee0261806737a32dc: Merge remote-tracking branch 'spice/spice.v72' into staging (2013-07-30 18:48:58 -0500) are available in the git repository at: git://git.kraxel.org/qemu usb.86 for you to fetch changes up to a14ff8a650b5943ee6221b952494661f7cb3b5e2: usb-redir: fix use-after-free (2013-08-01 13:03:42 +0200) Gerd Hoffmann (2): xhci: fix segfault usb-redir: fix use-after-free hw/usb/hcd-xhci.c |5 ++--- hw/usb/redirect.c |1 + 2 files changed, 3 insertions(+), 3 deletions(-)
Re: [Qemu-devel] [PATCH 0/4] dump-guest-memory: correct the vmcores
On Mon, 29 Jul 2013 16:37:12 +0200 Laszlo Ersek ler...@redhat.com wrote: (Apologies for the long To: list, I'm including everyone who participated in https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02607.html). Conceptually, the dump-guest-memory command works as follows: (a) pause the guest, (b) get a snapshot of the guest's physical memory map, as provided by qemu, (c) retrieve the guest's virtual mappings, as seen by the guest (this is where paging=true vs. paging=false makes a difference), (d) filter (c) as requested by the QMP caller, (e) write ELF headers, keying off (b) -- the guest's physmap -- and (d) -- the filtered guest mappings. (f) dump RAM contents, keying off the same (b) and (d), (g) unpause the guest (if necessary). Patch #1 affects step (e); specifically, how (d) is matched against (b), when paging is true, and the guest kernel maps more guest-physical RAM than it actually has. This can be done by non-malicious, clean-state guests (eg. a pristine RHEL-6.4 guest), and may cause libbfd errors due to PT_LOAD entries (coming directly from the guest page tables) exceeding the vmcore file's size. Patches #2 to #4 are independent of the paging option (or, more precisely, affect them equally); they affect (b). Currently input parameter (b), that is, the guest's physical memory map as provided by qemu, is implicitly represented by ram_list.blocks. As a result, steps and outputs dependent on (b) will refer to qemu-internal offsets. Unfortunately, this breaks when the guest-visible physical addresses diverge from the qemu-internal, RAMBlock based representation. This can happen eg. for guests 3.5 GB, due to the 32-bit PCI hole; see patch #4 for a diagram. Patch #2 introduces input parameter (b) explicitly, as a reasonably minimal map of guest-physical address ranges. (Minimality is not a hard requirement here, it just decreases the number of PT_LOAD entries written to the vmcore header.) Patch #3 populates this map. Patch #4 rebases the dump-guest-memory command to it, so that steps (e) and (f) work with guest-phys addresses. As a result, the crash utility can parse vmcores dumped for big x86_64 guests (paging=false). Applied to the qmp branch, thanks. Please refer to Red Hat Bugzilla 981582 https://bugzilla.redhat.com/show_bug.cgi?id=981582. Disclaimer: as you can tell from my progress in the RHBZ, I'm new to the memory API. The way I'm using it might be retarded. Laszlo Ersek (4): dump: clamp guest-provided mapping lengths to ramblock sizes dump: introduce GuestPhysBlockList dump: populate guest_phys_blocks dump: rebase from host-private RAMBlock offsets to guest-physical addresses include/sysemu/dump.h |4 +- include/sysemu/memory_mapping.h | 30 ++- dump.c | 171 +- memory_mapping.c| 174 +-- stubs/dump.c|3 +- target-i386/arch_dump.c | 10 ++- 6 files changed, 300 insertions(+), 92 deletions(-)
Re: [Qemu-devel] pvpanic device should not be automatically included as an internal device
On Thu, 2013-08-01 at 16:32 +0300, Michael S. Tsirkin wrote: On Thu, Aug 01, 2013 at 04:08:57PM +0300, Marcel Apfelbaum wrote: Hi, The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. Now what will happen on a cluster with hundreds of such VMs? If that cluster has a health monitoring service it may show all the VMs in a not healthy state. My point is that a device that requires a driver that is not inbox, should not be present by default. One possible solution is to add it manually with -device from command line. Any thoughts? Marcel Interesting. You are basically saying we should have a rule that no new builtin devices should be added without an explicit request from management interface? Basically, yes. The only builtin devices shall be devices that the operating systems know how to handle with the default drivers. Marcel
[Qemu-devel] [PATCH 1/2] xhci: fix segfault
Guest trying to reset a endpoint of a disconnected device resulted in
xhci trying to dereference uport while being NULL, thereby crashing
qemu. Fix that by adding a check. Drop unused dev variable while
touching that code bit.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/hcd-xhci.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7cbf813..ff5f681 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1429,7 +1429,6 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned
int slotid,
{
XHCISlot *slot;
XHCIEPContext *epctx;
-USBDevice *dev;
trace_usb_xhci_ep_reset(slotid, epid);
assert(slotid = 1 slotid = xhci-numslots);
@@ -1465,8 +1464,8 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned
int slotid,
ep |= 0x80;
}
-dev = xhci-slots[slotid-1].uport-dev;
-if (!dev) {
+if (!xhci-slots[slotid-1].uport ||
+!xhci-slots[slotid-1].uport-dev) {
return CC_USB_TRANSACTION_ERROR;
}
--
1.7.9.7
Re: [Qemu-devel] [PATCH v5 0/2] e1000: add interrupt mitigation support
Am 01.08.2013 11:38, schrieb Stefan Hajnoczi:
On Wed, Jul 31, 2013 at 03:39:05PM +0200, Vincenzo Maffione wrote:
Ok, but it's unclear how do you prefer to create and empty
PC_COMPAT_1_6 in Patch 1.
If you want to keep this declaration form
[...]
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_6,
{ /* end of list */ }
},
[...]
in the two pc_*_machine_v1_6 structs, I'm forced to define
#define PC_COMPAT_1_6 { /*empty*/ }
but then I can't extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as header
(like you guys do for PC_COMPAT_1_5 and PC_COMPAT_1_4), because
otherwise PC_COMPAT_1_6 would act as a premature terminator for
PC_COMPAT_1_5 (right?).
Should I extend PC_COMPAT_1_5 with PC_COMPAT_1_6 as a tail, or
should I avoid extending it in the Patch 1, and do the extension in
Patch 2 (when I have a non-empty PC_COMPAT_1_6)?
You are right, (GlobalProperty[]) {, {...}} is not valid syntax. In
that case I would switch PC_COMPAT_1_6 into the e1000 interrupt
mitigation patch. That way the patches are bisectable.
You can still introduce the QEMU 1.7 pc machine type as a separate
patch if you wish, but I no longer see a big win if PC_COMPAT_1_6 cannot
be isolated from the e1000 change.
Andreas: Do you agree to do everything in a single patch?
I see now that it wouldn't work with an empty macro (unless we drop the
,{} and then later have to remember to add it back, which may be even
worse; my main concern was having the property set in the actual patch
for bisecting and cherry-picking, so no objections from my side.
mst was the other candidate for needing compat_props for now-delayed ACPI.
Stefan, you haven't replied wrt VMXNET3 and MSI-X yet - that may be
another candidate if we can't break migration format as proposed.
But we can always introduce the same machine in several patches, we just
need to be careful with merging them to not get multiple 1.7 machines
and not to loose properties.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH for mst/pci] output nc-name in NIC_RX_FILTER_CHANGED event
On Thu, Aug 01, 2013 at 03:30:53PM +0200, Andreas Färber wrote:
Am 01.07.2013 04:55, schrieb Amos Kong:
On Wed, Jun 26, 2013 at 12:07:53PM +0200, Markus Armbruster wrote:
Amos Kong ak...@redhat.com writes:
On Mon, Jun 24, 2013 at 02:34:59PM +0800, Amos Kong wrote:
netclient 'name' entry in event is useful for management to know
which device is changed. n-netclient_name is not always set.
This patch changes to use nc-name. If we don't assign 'id',
qemu will set a generated name to nc-name.
IRC: mst akong, what do other events include? name or id?
I just checked QMP/qmp-event.txt, they all use 'device name'.
(eg: BLOCK_IO_ERROR, DEVICE_DELETED, DEVICE_TRAY_MOVED, BLOCK_JOB_*)
If we assign 'id' for -device, device name will be set to id.
Otherwise, a generated device name will set to some device.
DEVICE_DELETED uses device (the qdev ID) and path (the QOM path).
For reasons I don't understand, it sets path only when the device has
no qdev ID. That should be cleaned up.
The path are alwasy set.
example:
(have id)
path: /machine/peripheral-anon/vnet0/virtio-backend
You hopefully meant /machine/peripheral/vnet0/virtio-backend?
Otherwise we have a bug somewhere.
Just my typo in mail.
// -device virtio-net-pci,netdev=h1,id=vnet0 -netdev tap,id=h1
{
timestamp: {
seconds: 1375366321,
microseconds: 595727
},
event: NIC_RX_FILTER_CHANGED,
data: {
name: vnet0,
path: /machine/peripheral/vnet0/virtio-backend
}
}
Andreas
(no id)
path: /machine/peripheral-anon/device[0]/virtio-backend
It's enough to just use path to distinguish the changed device.
So we ignore this patch.
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
Amos.
[Qemu-devel] [v2][PATCH 1/1] cpus: use cpu_is_stopped efficiently
It makes more sense and simple later.
Signed-off-by: Tiejun Chen tiejun.c...@windriver.com
---
v1 - v2:
To optimize performance slightly, we can reorder the two conditions to
avoid the non-inline function call if cpu-stopped.
cpus.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cpus.c b/cpus.c
index c232265..7e76506 100644
--- a/cpus.c
+++ b/cpus.c
@@ -62,6 +62,11 @@
static CPUArchState *next_cpu;
+bool cpu_is_stopped(CPUState *cpu)
+{
+return cpu-stopped || !runstate_is_running();
+}
+
static bool cpu_thread_is_idle(CPUArchState *env)
{
CPUState *cpu = ENV_GET_CPU(env);
@@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
if (cpu-stop || cpu-queued_work_first) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return true;
}
if (!cpu-halted || qemu_cpu_has_work(cpu) ||
@@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
}
}
-bool cpu_is_stopped(CPUState *cpu)
-{
-return !runstate_is_running() || cpu-stopped;
-}
-
static void do_vm_stop(RunState state)
{
if (runstate_is_running()) {
@@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
if (cpu-stop) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return false;
}
return true;
--
1.7.9.5
Re: [Qemu-devel] [PATCH] cpus: use cpu_is_stopped efficiently
On 08/01/2013 07:38 PM, � wrote:
Hi,
Am 26.07.2013 10:47, schrieb Tiejun Chen:
It makes more sense and simple later.
Signed-off-by: Tiejun Chen tiejun.c...@windriver.com
---
cpus.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cpus.c b/cpus.c
index c232265..a997632 100644
--- a/cpus.c
+++ b/cpus.c
@@ -62,6 +62,11 @@
static CPUArchState *next_cpu;
+bool cpu_is_stopped(CPUState *cpu)
+{
+return !runstate_is_running() || cpu-stopped;
+}
+
static bool cpu_thread_is_idle(CPUArchState *env)
{
CPUState *cpu = ENV_GET_CPU(env);
To optimize performance slightly, I would suggest to reorder the two
conditions as they were below (avoiding the non-inline function call if
cpu-stopped).
Good idea.
Other than that it looks good to me, but no bugfix for 1.6.
If you send a v2 I can queue it on qom-cpu for the next merge window in
two weeks.
I already send this v2 just now.
CC'ing me would have made me review it earlier. ;) And as you may have
noticed, Avi is no longer with Red Hat, and Gleb and Paolo are
maintaining KVM parts, which there are none in this patch. See
MAINTAINERS file for the latest list.
Thanks for your information :)
Tiejun
Regards,
Andreas
@@ -69,7 +74,7 @@ static bool cpu_thread_is_idle(CPUArchState *env)
if (cpu-stop || cpu-queued_work_first) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return true;
}
if (!cpu-halted || qemu_cpu_has_work(cpu) ||
@@ -432,11 +437,6 @@ void cpu_synchronize_all_post_init(void)
}
}
-bool cpu_is_stopped(CPUState *cpu)
-{
-return !runstate_is_running() || cpu-stopped;
-}
-
static void do_vm_stop(RunState state)
{
if (runstate_is_running()) {
@@ -455,7 +455,7 @@ static bool cpu_can_run(CPUState *cpu)
if (cpu-stop) {
return false;
}
-if (cpu-stopped || !runstate_is_running()) {
+if (cpu_is_stopped(cpu)) {
return false;
}
return true;
Re: [Qemu-devel] [PATCH] monitor: fix parsing of big int
On Thu, 01 Aug 2013 07:52:17 -0600 Eric Blake ebl...@redhat.com wrote: On 08/01/2013 12:31 AM, Fam Zheng wrote: Fix it by calling strtoll instead, which will report ERANGE as expected. (HMP) block_set_io_throttle ide0-hd0 99 0 0 0 0 0 (HMP) block_set_io_throttle ide0-hd0 999 0 0 0 0 0 number too large (HMP) block_set_io_throttle ide0-hd0 0 0 0 0 0 number too large Your change causes this error message: (HMP) block_set_io_throttle ide0-hd0 - 0 0 0 0 0 number too large Does the too large mean in magnitude (correct message) or in value (misleading message, as any negative number is smaller in value than our minimum of 0)? Signed-off-by: Fam Zheng f...@redhat.com --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 5dc0aa9..7bfb469 100644 --- a/monitor.c +++ b/monitor.c @@ -3286,7 +3286,7 @@ static int64_t expr_unary(Monitor *mon) break; default: errno = 0; -n = strtoull(pch, p, 0); +n = strtoll(pch, p, 0); I'm worried that this will break callers that treat their argument as unsigned, and where the full range of unsigned input was desirable. At this point, it's probably safer to do a case-by-case analysis of all callers that use expr_unary() to decide which callers must reject negative values, instead of making the parser reject numbers that it previously accepted, thus changing the behavior of callers that treated the result as unsigned. Fam, what motivated this change? Is anyone entering such big numbers for block_set_io_throttle?
Re: [Qemu-devel] [PATCH v2 4/9] block: vhdx - log support struct and defines
On Wed, Jul 31, 2013 at 11:23:49PM -0400, Jeff Cody wrote:
@@ -318,6 +323,18 @@ typedef struct VHDXMetadataEntries {
uint16_t present;
} VHDXMetadataEntries;
+typedef struct VHDXLogEntries {
+uint64_t offset;
+uint64_t length;
+uint32_t head;
+uint32_t tail;
+} VHDXLogEntries;
+
+typedef struct VHDXLogEntryInfo {
+uint64_t sector_start;
+uint32_t desc_count;
+} VHDXLogEntryInfo;
An array of VHDXLogEntryInfo structs would be affected by alignment on
x86_64. a[1] != (void*)a + sizeof(VHDXLogEntryInfo).
IMO all on-disk structs should be QEMU_PACKED for safety.
[Qemu-devel] [PATCH 2/2] usb-redir: fix use-after-free
Reinitialize dev-cs to NULL after deleting it, to make sure it isn't used afterwards. Reported-by: Martin Cerveny m.cerv...@computer.org Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/redirect.c |1 + 1 file changed, 1 insertion(+) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 8b8c010..e3b9f32 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1334,6 +1334,7 @@ static void usbredir_handle_destroy(USBDevice *udev) USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev); qemu_chr_delete(dev-cs); +dev-cs = NULL; /* Note must be done after qemu_chr_close, as that causes a close event */ qemu_bh_delete(dev-chardev_close_bh); -- 1.7.9.7
[Qemu-devel] [PATCH] target-ppc: Add POWER7+ CPU model
This patch adds CPU PVR definition for POWER7+.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
target-ppc/cpu-models.c | 2 ++
target-ppc/cpu-models.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 9578ed8..c97c183 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -1143,6 +1143,8 @@
POWER7 v2.1)
POWERPC_DEF(POWER7_v2.3, CPU_POWERPC_POWER7_v23, POWER7,
POWER7 v2.3)
+POWERPC_DEF(POWER7P, CPU_POWERPC_POWER7P,POWER7,
+POWER7P)
POWERPC_DEF(POWER8_v1.0, CPU_POWERPC_POWER8_v10, POWER8,
POWER8 v1.0)
POWERPC_DEF(970, CPU_POWERPC_970,970,
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 01e488f..c3c78d1 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -556,6 +556,7 @@ enum {
CPU_POWERPC_POWER7_v20 = 0x003F0200,
CPU_POWERPC_POWER7_v21 = 0x003F0201,
CPU_POWERPC_POWER7_v23 = 0x003F0203,
+CPU_POWERPC_POWER7P= 0x004A0201,
CPU_POWERPC_POWER8_v10 = 0x004B0100,
CPU_POWERPC_970= 0x00390202,
CPU_POWERPC_970FX_v10 = 0x00391100,
--
1.8.3.2
Re: [Qemu-devel] [PATCH v2 2/9] block: vhdx - add header update capability.
On Thu, Aug 01, 2013 at 03:44:41PM +0200, Stefan Hajnoczi wrote:
On Wed, Jul 31, 2013 at 11:23:47PM -0400, Jeff Cody wrote:
@@ -212,6 +242,24 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size,
int crc_offset)
/*
+ * This generates a UUID that is compliant with the MS GUIDs used
+ * in the VHDX spec (and elsewhere).
+ *
+ * We can do this with uuid_generate if uuid.h is present,
+ * however not all systems have uuid and the generation is
+ * pretty straightforward for the DCE + random usage case
The talk of uuid.h not being available confuses me. The code has no
#ifdef and we do not define uuid_generate() ourselves.
Is this an outdated comment?
Yes, the comment is outdated from when I had my own UUID generation in
the patch - I missed removing the comment.
+/* Update the VHDX headers
+ *
+ * This follows the VHDX spec procedures for header updates.
+ *
+ * - non-current header is updated with largest sequence number
+ */
+static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s, bool
rw)
rw should be named generate_file_write_guid. If you call
vhdx_update_header() again later you do not need to update FileWriteGuid
again according to the spec. There's probably no harm in doing so but
the spec explicitly says If this is the first header update within the
session, use a new value for the FileWriteGuid.
This means that future calls to vhdx_update_header() in the same session
should set generate_file_write_guid to false. Therefore rw is not
the right name.
Yes, it is a bit misnamed. I'll change the name. However, redundant
generation is protected against with via vhdx_user_visible_write(),
introduced in patch 6, which is the only placed that calls it with rw
= true (soon to be generate_file_write_guid = true).
+{
+int ret = 0;
+int hdr_idx = 0;
+uint64_t header_offset = VHDX_HEADER1_OFFSET;
+
+VHDXHeader *active_header;
+VHDXHeader *inactive_header;
+VHDXHeader header_le;
+uint8_t *buffer;
+
+/* operate on the non-current header */
+if (s-curr_header == 0) {
+hdr_idx = 1;
+header_offset = VHDX_HEADER2_OFFSET;
+}
+
+active_header = s-headers[s-curr_header];
+inactive_header = s-headers[hdr_idx];
+
+inactive_header-sequence_number = active_header-sequence_number + 1;
+
+/* a new file guid must be generate before any file write, including
s/generate/generated/
Thanks
+ * headers */
+memcpy(inactive_header-file_write_guid, s-session_guid,
+ sizeof(MSGUID));
+
+/* a new data guid only needs to be generate before any guest-visible
+ * writes, so update it if the image is opened r/w. */
+if (rw) {
+vhdx_guid_generate(inactive_header-data_write_guid);
+}
+
+/* the header checksum is not over just the packed size of VHDXHeader,
+ * but rather over the entire 'reserved' range for the header, which is
+ * 4KB (VHDX_HEADER_SIZE). */
+
+buffer = qemu_blockalign(bs, VHDX_HEADER_SIZE);
+/* we can't assume the extra reserved bytes are 0 */
+ret = bdrv_pread(bs-file, header_offset, buffer, VHDX_HEADER_SIZE);
+if (ret 0) {
+goto exit;
+}
+/* overwrite the actual VHDXHeader portion */
+memcpy(buffer, inactive_header, sizeof(VHDXHeader));
+inactive_header-checksum = vhdx_update_checksum(buffer,
+ VHDX_HEADER_SIZE, 4);
+vhdx_header_le_export(inactive_header, header_le);
+bdrv_pwrite_sync(bs-file, header_offset, header_le,
sizeof(VHDXHeader));
This function can fail and it's a good indicator that future I/Os will
also be in trouble. Please propagate the error return.
You are right, that was unintentional.
@@ -801,8 +955,7 @@ static int vhdx_open(BlockDriverState *bs, QDict
*options, int flags)
}
if (flags BDRV_O_RDWR) {
-ret = -ENOTSUP;
-goto fail;
+vhdx_update_headers(bs, s, false);
Error handling is missing. At this point it looks like we cannot write
to the file. Propagating the error seems reasonable.
OK, I move the above change into when the write support is actually in
place.
[Qemu-devel] [PATCH 4/8] [PATCH RFC v3] s390-qemu: cpu hotplug - Storage key global access
From: Jason J. Herne jjhe...@us.ibm.com
Introduces global access to storage key data so we can set it for each cpu in
the S390 cpu initialization routine.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/s390-virtio-ccw.c |5 ++---
hw/s390x/s390-virtio.c | 21 -
hw/s390x/s390-virtio.h |2 +-
target-s390x/cpu.h |4
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index aebbbf1..b469960 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -65,7 +65,6 @@ static void ccw_init(QEMUMachineInitArgs *args)
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
int shift = 0;
-uint8_t *storage_keys;
int ret;
VirtualCssBus *css_bus;
@@ -94,10 +93,10 @@ static void ccw_init(QEMUMachineInitArgs *args)
memory_region_add_subregion(sysmem, 0, ram);
/* allocate storage keys */
-storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+s390_alloc_storage_keys(my_ram_size);
/* init CPUs */
-s390_init_cpus(args-cpu_model, storage_keys);
+s390_init_cpus(args-cpu_model);
if (kvm_enabled()) {
kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 439d732..21e9124 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -123,6 +123,18 @@ static void s390_virtio_register_hcalls(void)
s390_virtio_hcall_set_status);
}
+static uint8_t *storage_keys;
+
+uint8_t *s390_get_storage_keys(void)
+{
+return storage_keys;
+}
+
+void s390_alloc_storage_keys(ram_addr_t ram_size)
+{
+storage_keys = g_malloc0(ram_size / TARGET_PAGE_SIZE);
+}
+
/*
* The number of running CPUs. On s390 a shutdown is the state of all CPUs
* being either stopped or disabled (for interrupts) waiting. We have to
@@ -176,7 +188,7 @@ void s390_init_ipl_dev(const char *kernel_filename,
qdev_init_nofail(dev);
}
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
+void s390_init_cpus(const char *cpu_model)
{
int i;
@@ -196,7 +208,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t
*storage_keys)
ipi_states[i] = cpu;
cs-halted = 1;
cpu-env.exception_index = EXCP_HLT;
-cpu-env.storage_keys = storage_keys;
+cpu-env.storage_keys = s390_get_storage_keys();
}
}
@@ -231,7 +243,6 @@ static void s390_init(QEMUMachineInitArgs *args)
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
int shift = 0;
-uint8_t *storage_keys;
void *virtio_region;
hwaddr virtio_region_len;
hwaddr virtio_region_start;
@@ -270,10 +281,10 @@ static void s390_init(QEMUMachineInitArgs *args)
virtio_region_len);
/* allocate storage keys */
-storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+s390_alloc_storage_keys(my_ram_size);
/* init CPUs */
-s390_init_cpus(args-cpu_model, storage_keys);
+s390_init_cpus(args-cpu_model);
/* Create VirtIO network adapters */
s390_create_virtio_net((BusState *)s390_bus, virtio-net-s390);
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index 5c405e7..c1cb042 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -20,7 +20,7 @@
typedef int (*s390_virtio_fn)(const uint64_t *args);
void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys);
+void s390_init_cpus(const char *cpu_model);
void s390_init_ipl_dev(const char *kernel_filename,
const char *kernel_cmdline,
const char *initrd_filename,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 65bef86..877eac7 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -374,6 +374,10 @@ static inline void kvm_s390_interrupt_internal(S390CPU
*cpu, int type,
{
}
#endif
+
+uint8_t *s390_get_storage_keys(void);
+void s390_alloc_storage_keys(ram_addr_t ram_size);
+
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
void s390_add_running_cpu(S390CPU *cpu);
unsigned s390_del_running_cpu(S390CPU *cpu);
--
1.7.10.4
