[Qemu-devel] [PATCH] target-i386/cpu.c: Fix two error output indentation

2014-07-28 Thread Chen Fan
Signed-off-by: Chen Fan 
---
 target-i386/cpu.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6d008ab..217500c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1716,9 +1716,9 @@ static void x86_set_hv_spinlocks(Object *obj, Visitor *v, 
void *opaque,
 
 if (value < min || value > max) {
 error_setg(errp, "Property %s.%s doesn't take value %" PRId64
-  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
-  object_get_typename(obj), name ? name : "null",
-  value, min, max);
+   " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
+   object_get_typename(obj), name ? name : "null",
+   value, min, max);
 return;
 }
 cpu->hyperv_spinlock_attempts = value;
@@ -1808,8 +1808,8 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
*features,
 }
 if (numvalue < min) {
 error_report("hv-spinlocks value shall always be >= 0x%x"
-", fixup will be removed in future versions",
-min);
+ ", fixup will be removed in future versions",
+ min);
 numvalue = min;
 }
 snprintf(num, sizeof(num), "%" PRId32, numvalue);
-- 
1.9.3




Re: [Qemu-devel] [PATCH for-2.1 1/2] migration: load smaller RAMBlock to a bigger one if permitted

2014-07-28 Thread Igor Mammedov
On Fri, 25 Jul 2014 19:56:40 +0200
Laszlo Ersek  wrote:

> On 07/25/14 17:48, Igor Mammedov wrote:
> 
> > Add API to mark memory region as extend-able on migration,
> > to allow migration code to load smaller RAMBlock into
> > a bigger one on destination QEMU instance.
> >
> > This will allow to fix broken migration from QEMU 1.7/2.0 to
> > QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
> > versions by marking ACPI tables ROM blob as extend-able.
> > So that smaller tables from previos version could be always
> 
> ("previous")
> 
> > migrated to a bigger rom blob on new version.
> >
> > Credits-for-idea: Michael S. Tsirkin 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  arch_init.c | 19 ++-
> >  exec.c  | 15 +--
> >  include/exec/cpu-all.h  | 15 ++-
> >  include/exec/memory.h   | 10 ++
> >  include/exec/ram_addr.h |  1 +
> >  memory.c|  5 +
> >  6 files changed, 53 insertions(+), 12 deletions(-)
> 
> (Please pass the -O flag to git-format-patch, with an order file that
> moves header files (*.h) to the front. Header hunks should lead in a
> patch. Thanks.)
> 
> > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> > index 6593be1..248c075 100644
> > --- a/include/exec/ram_addr.h
> > +++ b/include/exec/ram_addr.h
> > @@ -33,6 +33,7 @@ void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
> >  void *qemu_get_ram_ptr(ram_addr_t addr);
> >  void qemu_ram_free(ram_addr_t addr);
> >  void qemu_ram_free_from_ptr(ram_addr_t addr);
> > +void qemu_ram_set_extendable_on_migration(ram_addr_t addr);
> >
> >  static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
> >   ram_addr_t length,
> 
> (Ugh. The declarations (prototypes) of qemu_ram_*() functions are
> scattered between "ram_addr.h" and "cpu-common.h" (both under
> include/exec). I can't see why that is a good thing (the function
> definitions all seem to be in exec.c).)
Trying to avoid putting stuff to global cpu-common.h, I've put
definition in ram_addr.h. The rest should be moved from cpu-common.h
to ram_addr.h when 2.2 is open.

[...]

> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 8ddaf35..812f8b5 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -1071,11 +1071,20 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> > version_id)
> >
> >  QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> >  if (!strncmp(id, block->idstr, sizeof(id))) {
> > -if (block->length != length) {
> > -error_report("Length mismatch: %s: " 
> > RAM_ADDR_FMT
> > - " in != " RAM_ADDR_FMT, id, 
> > length,
> > - block->length);
> > -ret =  -EINVAL;
> > +if (block->flags & RAM_EXTENDABLE_ON_MIGRATE) {
> > +if (block->length < length) {
> > +error_report("Length too big: %s: 
> > "RAM_ADDR_FMT
> 
> missing space between '"' and RAM_ADDR_FMT (for readability)
> 
> 
> > + " > " RAM_ADDR_FMT, id, 
> > length,
> 
> you dropped the important word "in" here (cf. "in !=" above). That word
> explains it to the user that the incoming size (stored in "length") is
> too big.
> 
> > + block->length);
> > +ret =  -EINVAL;
> > +}
> > +} else {
> > +if (block->length != length) {
> > +error_report("Length mismatch: %s: 
> > "RAM_ADDR_FMT
> 
> missing space between '"' and RAM_ADDR_FMT (for readability)
> 
> > + " in != " RAM_ADDR_FMT, id, 
> > length,
> > + block->length);
> > +ret =  -EINVAL;
> > +}
> >  }
> >  break;
> >  }
> 
> Can you please explain where it is ensured that the non-overwritten part
> of a greater RAMBlock is zero-filled? As far as I can see:
> 
> main() [vl.c]
>   qemu_run_machine_init_done_notifiers() [vl.c]
> notifier_list_notify() [util/notify.c]
>   pc_guest_info_machine_done() [hw/i386/pc.c]
> acpi_setup() [hw/i386/acpi-build.c]
>   acpi_add_rom_blob() [hw/i386/acpi-build.c]
> rom_add_blob() [hw/core/loader.c]
>   rom_set_mr() [hw/core/loader.c]
> memory_region_init_ram() [memory.c]
>   qemu_ram_alloc() [exec.c]
> memcpy()  <-- populates it
>   fw_cfg_add_file_callback() [hw/nvram/fw_cfg.c]
> fw_cfg_add_bytes_read_cal

Re: [Qemu-devel] [RFC PATCH v2 00/49] Series short description

2014-07-28 Thread Pavel Dovgaluk
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Il 17/07/2014 13:01, Pavel Dovgalyuk ha scritto:
> > This set of patches is related to the reverse execution and deterministic
> > replay of qemu execution  Our implementation of deterministic replay can
> > be used for deterministic and reverse debugging of guest code through gdb
> > remote interface.
> >
> > Execution recording writes non-deterministic events log, which can be later
> > used for replaying the execution anywhere and for unlimited number of times.
> > It also supports checkpointing for faster rewinding during reverse 
> > debugging.
> > Execution replaying reads the log and replays all non-deterministic events
> > including external input, hardware clocks, and interrupts.
> 
> From a first look:
> 
> - patches 2 to 13 probably should try to use subsections, so that VMs
> that do not use the devices try not to save the extra data and keep
> backwards migration compatibility (at least try to)

 Could you give me and example?
 As I know, subsection is loaded when some predicate function returns true. How 
can I 
construct such a function for integratorcp module? What kind of condition 
should it check?
 In this module I just added missed vmstates (it does not saved/restored at all
by the master version).

> - patch 16 should also use subsections, and perhaps apply to all other
> CPUs too?

 We implemented replay only for i386 and ARM. If we'll change other targets, it 
will not
add record/replay capabilities to them, but can confuse the reviewers.

> - patches 23-24-25 perhaps could try using icount, like Konrad's patch do?

 Using faster icount (like in Konrad's patches) is the our next aim. It 
obviously will 
increase the speed of recording process. But now I submitted slower, but more 
conservative
version of icount which we had already tested.

> - patch 27 makes sense but VIRTUAL is used to skip blinking when the VM
> is stopped

 Right, this is kind of hack. I haven't found better solution yet.

> - the others I haven't yet looked at, but they look like something that
> would bitrot really, really fast.  For patch 33, I think changing
> INSERT_HEAD to INSERT_TAIL would be just fine, and I wonder if it's the
> same for other patches here.  How do you plan on testing them and
> keeping them up to date?

 We're constantly keeping these patches up to date, because we are using 
deterministic replay
and reverse debugging for solving our tasks. We re-test all the features when 
pulling new
patches from the master branch.

Pavel Dovgalyuk

> 
> > Reverse execution has the following features:
> >  * Deterministically replays whole system execution and all contents of the 
> > memory,
> >state of the hadrware devices, clocks, and screen of the VM.
> >  * Writes execution log into the file for latter replaying for multiple 
> > times
> >on different machines.
> >  * Supports i386, x86_64, and ARM hardware platforms.
> >  * Performs deterministic replay of all operations with keyboard, mouse, 
> > network adapters,
> >audio devices, serial interfaces, and physical USB devices connected to 
> > the emulator.
> >  * Provides support for gdb reverse debugging commands like reverse-step 
> > and reverse-
> continue.
> >  * Supports auto-checkpointing for convenient reverse debugging.
> >
> > Usage of the record/replay:
> >  * First, record the execution, by adding '-record fname=replay.bin' to the
> >command line.
> >  * Then you can replay it for the multiple times by using another command
> >line option: '-replay fname=replay.bin'
> >  * Virtual machine should have at least one virtual disk, which is used to
> >store checkpoints. If you want to enable automatic checkpointing, simply
> >add ',period=XX' to record options, where XX is the checkpointing period
> >in seconds.
> >  * Using of the network adapters in record/replay mode is possible with
> >the following command-line options:
> >- '-net user' (or another host adapter) in record mode
> >- '-net replay' in replay mode. Every host network adapter should be
> >  replaced by 'replay' when replaying the execution.
> >  * Reverse debugging can be used through gdb remote interface.
> >reverse-stepi and reverse-continue commands are supported. Other reverse
> >commands should also work, because they reuse these ones.
> >  * Monitor is extended by the following commands:
> >- replay_info - prints information about replay mode and current step
> >  (number of instructions executed)
> >- replay_break - sets "breakpoint" at the specified instructions count.
> >- replay_seek - rewinds (using the checkpoints, if possible) to the
> >  specified step of replay log.
> >
> > Paper with short description of deterministic replay implementation:
> > http://www.computer.org/csdl/proceedings/csmr/2012/4666/00/4666a553-abs.html
> >
> > Modifications of qemu include:
> >  * adding missed fields of the vir

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.2] spapr: add host Linux version information to device tree

2014-07-28 Thread Alexander Graf


> Am 28.07.2014 um 08:47 schrieb Alexey Kardashevskiy :
> 
>> On 07/24/2014 11:15 PM, Alexander Graf wrote:
>> 
>>> On 18.07.14 06:31, cyril...@gmail.com wrote:
>>> It may prove useful know which Linux distribution version the host machine
>>> is running when an issue in the guest arises but a user cannot access
>>> the host.
>>> 
>>> Signed-off-by: Cyril Bur 
>>> ---
>>>  hw/ppc/spapr.c   |  8 +++
>>>  target-ppc/kvm.c | 62
>>> 
>>>  target-ppc/kvm_ppc.h |  6 +
>>>  3 files changed, 76 insertions(+)
>>> 
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 6b48a26..391d47a 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>  _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
>>>  g_free(buf);
>>>  +/*
>>> + * Add info to the guest FDT to tell it what linux the host is
>>> + */
>>> +if (kvmppc_get_linux_host(&buf)) {
>>> +_FDT((fdt_property_string(fdt, "linux,host", buf)));
>> 
>> Is this even specified in sPAPR?
> 
> 
> PAPR does not know about any "linux,xxx" properties.
> 
>> 
>>> +g_free(buf);
>>> +}
>>> +
>>>  _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>>>  _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>>>  diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 8c9e79c..95e0970 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1415,6 +1415,68 @@ bool kvmppc_get_host_model(char **value)
>>>  return g_file_get_contents("/proc/device-tree/model", value, NULL,
>>> NULL);
>>>  }
>>>  +bool kvmppc_get_linux_host(char **value)
>>> +{
>>> +FILE *f;
>>> +int i;
>>> +char line[512];
>>> +const char *names[] = {"NAME", "VERSION", "BUILD_ID"};
>>> +bool names_found[ARRAY_SIZE(names)] = { 0 };
>>> +GString *output = NULL;
>>> +f = fopen("/etc/os-release", "r");
>> 
>> A few comments:
>> 
>>  1) Why would anyone care?
> 
> 
> Useful debug info when the host is not reachable.
> 
> 
>>  2) I'm not sure KVM is the right decision maker on whether we want this
>> exposed or not.
> 
> Good point, there is no reason not to show this info in TCG.
> 
>> After all, the files you read here are available on an x86
>> host just as well
> 
> Does qemu-x86 has any way to pass information like this to the guest?

Yes and no. It has fw_cfg where we could put it and it has DSST generation 
where we could also put it I guess. Just like here, there's no standardized way 
to expose it though.

Alex

> 
> 
>>  3) Use glib functions to read files
>> 
>> 
>> Alex
> 
> 
> -- 
> Alexey



Re: [Qemu-devel] [PATCH for-2.1 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

2014-07-28 Thread Igor Mammedov
On Fri, 25 Jul 2014 19:37:41 +0200
Paolo Bonzini  wrote:

> Il 25/07/2014 17:48, Igor Mammedov ha scritto:
> > It fixes migration failure for machine type pc-i440fx-1.7 from
> > QEMU 1.7/2.0 to QEMU 2.1
> > 
> > Migration fails due to ACPI tables size grows across 1.7-2.1
> > versions. That causes ACPI tables ROM blob to change its size
> > differently for the same configurations on different QEMU versions.
> > As result migration code bails out when attempting to load
> > smaller ROM blob into a bigger one on a newer QEMU.
> > To trigger failure it's enough to add pci-bridge device to QEMU CLI
> > 
> > Marking ACPI tables ROM blob as extend-able on migration allows
> > QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
> > forward migration failure introduced since 2.0 which affects
> > only configurations that cause ACPI ROM blob size change.
> 
> This works in this case, and it is more friendly to downstreams indeed.
>  It also is simpler, at least on the surface.  I think the ramifications
> could be a bit larger than with my own patches, but still I guess it's
> more appropriate at this point of the release cycle.
> 
> It doesn't handle the case of ACPI tables that shrink, which can happen
> as well.  I guess if this ever happens we can just hard-code the table
> size of the "old" versions to something big enough (64K?) and keep using
> fine-grained sizing.
That was intentionally omitted in here so far size only goes up from 1.7 to
2.1. My though was that can enforce minimum size later during 2.2 cycle
Anyway, I'll think more about it, and maybe post additional patch
on top of this to set minimum size if I find a reason for it to be in 2.1.


> 
> I'd like a day or two to mull about it, but I have it even if the
> patches are applied.  Peter, feel free to go ahead with Igor's patches.
> 
> Paolo
> 
> 




[Qemu-devel] [PATCH for-2.1 v2 0/2] Fix migration failure due to ACPI tables size changes

2014-07-28 Thread Igor Mammedov

Changes since v2:
  - addressed Laszlo's comments
 * fixing typos, rewording comments
 * dropping enum-ification of RAMBlock flags
 * adding zeroing out destination ramblock
 * replacing 'if' with assert() 

Changing the ACPI table size causes migration to break, and the memory
hotplug work opened our eyes on how horribly we were breaking things in
2.0 already.

To trigger issue start
  QEMU-1.7 with -M pc-i440fx-1.7 -device pci-bridge,chassis_nr=1
and try to migrate to QEMU-2.1 or QEMU-2.0 as result target will fail with:
  qemu-system-x86_64: Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000

This fix allows target QEMU to load smaller RAMBlock into a bigger one
and fixes regression which was introduced since 2.0, allowing
forward migration from 1.7/2.0 to 2.1
Fix is also suitable for stable-2.0.


Igor Mammedov (2):
  migration: load smaller RAMBlock to a bigger one if permitted
  acpi: mark ACPI tables ROM blob as extend-able on migration

 arch_init.c | 22 +-
 exec.c  |  8 
 hw/core/loader.c|  6 +-
 hw/i386/acpi-build.c|  2 +-
 include/exec/memory.h   | 11 +++
 include/exec/ram_addr.h |  3 +++
 include/hw/loader.h |  5 +++--
 memory.c|  5 +
 8 files changed, 53 insertions(+), 9 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH for-2.1 v2 1/2] migration: load smaller RAMBlock to a bigger one if permitted

2014-07-28 Thread Igor Mammedov
Add API to mark memory region as extend-able on migration,
to allow migration code to load smaller RAMBlock into
a bigger one on destination QEMU instance.

This will allow to fix broken migration from QEMU 1.7/2.0 to
QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
versions by marking ACPI tables ROM blob as extend-able.
So that smaller tables from previous version could be always
migrated to a bigger rom blob on new version.

Credits-for-idea: Michael S. Tsirkin 
Signed-off-by: Igor Mammedov 
---
v2:
  fixed patch as suggested by Laszlo
---
 arch_init.c | 22 +-
 exec.c  |  8 
 include/exec/memory.h   | 11 +++
 include/exec/ram_addr.h |  3 +++
 memory.c|  5 +
 5 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 8ddaf35..2c0c238 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1071,11 +1071,23 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 if (!strncmp(id, block->idstr, sizeof(id))) {
-if (block->length != length) {
-error_report("Length mismatch: %s: " RAM_ADDR_FMT
- " in != " RAM_ADDR_FMT, id, length,
- block->length);
-ret =  -EINVAL;
+if (block->flags & RAM_EXTENDABLE_ON_MIGRATE) {
+if (block->length < length) {
+error_report("Length too big: %s: " 
RAM_ADDR_FMT
+ " in > " RAM_ADDR_FMT, id, length,
+ block->length);
+ret =  -EINVAL;
+} else {
+memset(block->host, 0, block->length);
+}
+} else {
+if (block->length != length) {
+error_report("Length mismatch: %s: "
+ RAM_ADDR_FMT " in != "
+ RAM_ADDR_FMT,
+ id, length, block->length);
+ret =  -EINVAL;
+}
 }
 break;
 }
diff --git a/exec.c b/exec.c
index 765bd94..02536f8e 100644
--- a/exec.c
+++ b/exec.c
@@ -1214,6 +1214,14 @@ void qemu_ram_unset_idstr(ram_addr_t addr)
 }
 }
 
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr)
+{
+RAMBlock *block = find_ram_block(addr);
+
+assert(block != NULL);
+block->flags |= RAM_EXTENDABLE_ON_MIGRATE;
+}
+
 static int memory_try_enable_merging(void *addr, size_t len)
 {
 if (!qemu_opt_get_bool(qemu_get_machine_opts(), "mem-merge", true)) {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..f96ddbb 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -894,6 +894,17 @@ bool memory_region_present(MemoryRegion *container, hwaddr 
addr);
 bool memory_region_is_mapped(MemoryRegion *mr);
 
 /**
+ * memory_region_permit_extendable_migration: marks #MemoryRegion
+ * as extendable on migration, allowing the migration code to load
+ * source memory block of smaller size than destination memory block
+ * at migration time
+ *
+ * @mr: a #MemoryRegion whose #RAMBlock should be marked as
+ *  extendable on migration
+ */
+void memory_region_permit_extendable_migration(MemoryRegion *mr);
+
+/**
  * memory_region_find: translate an address/size relative to a
  * MemoryRegion into a #MemoryRegionSection.
  *
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 6593be1..7a6b782 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -34,6 +34,9 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
 void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
+#define RAM_EXTENDABLE_ON_MIGRATE (1U << 31)
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr);
+
 static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
  ram_addr_t length,
  unsigned client)
diff --git a/memory.c b/memory.c
index 64d7176..744c746 100644
--- a/memory.c
+++ b/memory.c
@@ -1791,6 +1791,11 @@ bool memory_region_is_mapped(MemoryRegion *mr)
 return mr->container ? true : false;
 }
 
+void memory_region_permit_extendable_migration(MemoryRegion *mr)
+{
+qemu_ram_set_extendable_on_migration(mr->ram_addr);
+}
+
 MemoryRegionSection memory_region_find(MemoryRegion *mr,
hwaddr addr, uint64_t size)
 {
-- 
1.8.3.1




[Qemu-devel] [PATCH for-2.1 v2 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

2014-07-28 Thread Igor Mammedov
It fixes migration failure for machine type pc-i440fx-1.7 from
QEMU 1.7/2.0 to QEMU 2.1

Migration fails due to ACPI tables size grows across 1.7-2.1
versions. That causes ACPI tables ROM blob to change its size
differently for the same configurations on different QEMU versions.
As result migration code bails out when attempting to load
smaller ROM blob into a bigger one on a newer QEMU.
To trigger failure it's enough to add pci-bridge device to QEMU CLI

Marking ACPI tables ROM blob as extend-able on migration allows
QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
forward migration failure introduced since 2.0 which affects
only configurations that cause ACPI ROM blob size change.

Signed-off-by: Igor Mammedov 
Reviewed-by: Laszlo Ersek 
---
 hw/core/loader.c | 6 +-
 hw/i386/acpi-build.c | 2 +-
 include/hw/loader.h  | 5 +++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..d09b894 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -722,7 +722,8 @@ err:
 
 void *rom_add_blob(const char *name, const void *blob, size_t len,
hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque)
+   FWCfgReadCallback fw_callback, void *callback_opaque,
+   bool extendable)
 {
 Rom *rom;
 void *data = NULL;
@@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, 
size_t len,
 
 if (rom_file_has_mr) {
 data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+if (extendable) {
+memory_region_permit_extendable_migration(rom->mr);
+}
 } else {
 data = rom->data;
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..651c06b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState 
*build_state, GArray *blob,
const char *name)
 {
 return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
-acpi_build_update, build_state);
+acpi_build_update, build_state, true);
 }
 
 static const VMStateDescription vmstate_acpi_build = {
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..e5a24ac 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
  bool option_rom);
 void *rom_add_blob(const char *name, const void *blob, size_t len,
hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque);
+   FWCfgReadCallback fw_callback, void *callback_opaque,
+   bool extendable);
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
 size_t romsize, hwaddr addr);
 int rom_load_all(void);
@@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)  \
 rom_add_file(_f, NULL, _a, _i, false)
 #define rom_add_blob_fixed(_f, _b, _l, _a)  \
-rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
+rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
 
 #define PC_ROM_MIN_VGA 0xc
 #define PC_ROM_MIN_OPTION  0xc8000
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] target-i386/cpu.c: Fix two error output indentation

2014-07-28 Thread Igor Mammedov
On Mon, 28 Jul 2014 15:13:06 +0800
Chen Fan  wrote:

> Signed-off-by: Chen Fan 
> ---
>  target-i386/cpu.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 6d008ab..217500c 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1716,9 +1716,9 @@ static void x86_set_hv_spinlocks(Object *obj, Visitor 
> *v, void *opaque,
>  
>  if (value < min || value > max) {
>  error_setg(errp, "Property %s.%s doesn't take value %" PRId64
> -  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> -  object_get_typename(obj), name ? name : "null",
> -  value, min, max);
> +   " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> +   object_get_typename(obj), name ? name : "null",
> +   value, min, max);
>  return;
>  }
>  cpu->hyperv_spinlock_attempts = value;
> @@ -1808,8 +1808,8 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
> *features,
>  }
>  if (numvalue < min) {
>  error_report("hv-spinlocks value shall always be >= 0x%x"
> -", fixup will be removed in future versions",
> -min);
> + ", fixup will be removed in future 
> versions",
> + min);
>  numvalue = min;
>  }
>  snprintf(num, sizeof(num), "%" PRId32, numvalue);

CCed qemu-triv...@nongnu.org

Reviewed-by: Igor Mammedov 



Re: [Qemu-devel] [PATCH for-2.1 1/2] migration: load smaller RAMBlock to a bigger one if permitted

2014-07-28 Thread Laszlo Ersek
On 07/28/14 09:40, Igor Mammedov wrote:
> On Fri, 25 Jul 2014 19:56:40 +0200
> Laszlo Ersek  wrote:
> 
>> On 07/25/14 17:48, Igor Mammedov wrote:
>>
>>> Add API to mark memory region as extend-able on migration,
>>> to allow migration code to load smaller RAMBlock into
>>> a bigger one on destination QEMU instance.
>>>
>>> This will allow to fix broken migration from QEMU 1.7/2.0 to
>>> QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
>>> versions by marking ACPI tables ROM blob as extend-able.
>>> So that smaller tables from previos version could be always
>>
>> ("previous")
>>
>>> migrated to a bigger rom blob on new version.
>>>
>>> Credits-for-idea: Michael S. Tsirkin 
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>>  arch_init.c | 19 ++-
>>>  exec.c  | 15 +--
>>>  include/exec/cpu-all.h  | 15 ++-
>>>  include/exec/memory.h   | 10 ++
>>>  include/exec/ram_addr.h |  1 +
>>>  memory.c|  5 +
>>>  6 files changed, 53 insertions(+), 12 deletions(-)
>>
>> (Please pass the -O flag to git-format-patch, with an order file that
>> moves header files (*.h) to the front. Header hunks should lead in a
>> patch. Thanks.)
>>
>>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>>> index 6593be1..248c075 100644
>>> --- a/include/exec/ram_addr.h
>>> +++ b/include/exec/ram_addr.h
>>> @@ -33,6 +33,7 @@ void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
>>>  void *qemu_get_ram_ptr(ram_addr_t addr);
>>>  void qemu_ram_free(ram_addr_t addr);
>>>  void qemu_ram_free_from_ptr(ram_addr_t addr);
>>> +void qemu_ram_set_extendable_on_migration(ram_addr_t addr);
>>>
>>>  static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
>>>   ram_addr_t length,
>>
>> (Ugh. The declarations (prototypes) of qemu_ram_*() functions are
>> scattered between "ram_addr.h" and "cpu-common.h" (both under
>> include/exec). I can't see why that is a good thing (the function
>> definitions all seem to be in exec.c).)
> Trying to avoid putting stuff to global cpu-common.h, I've put
> definition in ram_addr.h. The rest should be moved from cpu-common.h
> to ram_addr.h when 2.2 is open.
> 
> [...]
> 
>>>
>>> diff --git a/arch_init.c b/arch_init.c
>>> index 8ddaf35..812f8b5 100644
>>> --- a/arch_init.c
>>> +++ b/arch_init.c
>>> @@ -1071,11 +1071,20 @@ static int ram_load(QEMUFile *f, void *opaque, int 
>>> version_id)
>>>
>>>  QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>>>  if (!strncmp(id, block->idstr, sizeof(id))) {
>>> -if (block->length != length) {
>>> -error_report("Length mismatch: %s: " 
>>> RAM_ADDR_FMT
>>> - " in != " RAM_ADDR_FMT, id, 
>>> length,
>>> - block->length);
>>> -ret =  -EINVAL;
>>> +if (block->flags & RAM_EXTENDABLE_ON_MIGRATE) {
>>> +if (block->length < length) {
>>> +error_report("Length too big: %s: 
>>> "RAM_ADDR_FMT
>>
>> missing space between '"' and RAM_ADDR_FMT (for readability)
>>
>>
>>> + " > " RAM_ADDR_FMT, id, 
>>> length,
>>
>> you dropped the important word "in" here (cf. "in !=" above). That word
>> explains it to the user that the incoming size (stored in "length") is
>> too big.
>>
>>> + block->length);
>>> +ret =  -EINVAL;
>>> +}
>>> +} else {
>>> +if (block->length != length) {
>>> +error_report("Length mismatch: %s: 
>>> "RAM_ADDR_FMT
>>
>> missing space between '"' and RAM_ADDR_FMT (for readability)
>>
>>> + " in != " RAM_ADDR_FMT, id, 
>>> length,
>>> + block->length);
>>> +ret =  -EINVAL;
>>> +}
>>>  }
>>>  break;
>>>  }
>>
>> Can you please explain where it is ensured that the non-overwritten part
>> of a greater RAMBlock is zero-filled? As far as I can see:
>>
>> main() [vl.c]
>>   qemu_run_machine_init_done_notifiers() [vl.c]
>> notifier_list_notify() [util/notify.c]
>>   pc_guest_info_machine_done() [hw/i386/pc.c]
>> acpi_setup() [hw/i386/acpi-build.c]
>>   acpi_add_rom_blob() [hw/i386/acpi-build.c]
>> rom_add_blob() [hw/core/loader.c]
>>   rom_set_mr() [hw/core/loader.c]
>> memory_region_init_ram() [memory.c]
>>   qemu_ram_alloc() [exec.c]
>> memcpy()  <-- populates it
>>   fw_c

Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address

2014-07-28 Thread Paolo Bonzini
Il 27/07/2014 08:57, Jan Kiszka ha scritto:
> From: Jan Kiszka 
> 
> According to ICH9 spec, the MSI capability is located at 0x60. This is
> important for guest drivers that do not parse the capability chain and
> use absolute addresses instead.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  hw/audio/intel-hda.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index aa49b47..09c4118 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci)
>"intel-hda", 0x4000);
>  pci_register_bar(&d->pci, 0, 0, &d->mmio);
>  if (d->msi) {
> -msi_init(&d->pci, 0x50, 1, true, false);
> +msi_init(&d->pci, 0x60, 1, true, false);
>  }
> 
>  hda_codec_bus_init(DEVICE(pci), &d->codecs, sizeof(d->codecs),
> 

Does this need a compat property?

Paolo



Re: [Qemu-devel] [PATCH] pci: Use bus master address space for delivering MSI/MSI-X messages

2014-07-28 Thread Paolo Bonzini
Il 27/07/2014 09:08, Jan Kiszka ha scritto:
> From: Jan Kiszka 
> 
> The spec says (and real HW confirms this) that, if the bus master bit
> is 0, the device will not generate any PCI accesses. MSI and MSI-X
> messages fall among these, so we should use the corresponding address
> space to deliver them. This will prevent delivery if bus master support
> is disabled.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  hw/pci/msi.c  | 2 +-
>  hw/pci/msix.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> index a4a3040..52d2313 100644
> --- a/hw/pci/msi.c
> +++ b/hw/pci/msi.c
> @@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
> "notify vector 0x%x"
> " address: 0x%"PRIx64" data: 0x%"PRIx32"\n",
> vector, msg.address, msg.data);
> -stl_le_phys(&address_space_memory, msg.address, msg.data);
> +stl_le_phys(&dev->bus_master_as, msg.address, msg.data);
>  }
>  
>  /* Normally called by pci_default_write_config(). */
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index 5c49bfc..20ae476 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
>  
>  msg = msix_get_message(dev, vector);
>  
> -stl_le_phys(&address_space_memory, msg.address, msg.data);
> +stl_le_phys(&dev->bus_master_as, msg.address, msg.data);
>  }
>  
>  void msix_reset(PCIDevice *dev)
> 

Reviewed-by: Paolo Bonzini 



Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address

2014-07-28 Thread Jan Kiszka
On 2014-07-28 10:11, Paolo Bonzini wrote:
> Il 27/07/2014 08:57, Jan Kiszka ha scritto:
>> From: Jan Kiszka 
>>
>> According to ICH9 spec, the MSI capability is located at 0x60. This is
>> important for guest drivers that do not parse the capability chain and
>> use absolute addresses instead.
>>
>> Signed-off-by: Jan Kiszka 
>> ---
>>  hw/audio/intel-hda.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
>> index aa49b47..09c4118 100644
>> --- a/hw/audio/intel-hda.c
>> +++ b/hw/audio/intel-hda.c
>> @@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci)
>>"intel-hda", 0x4000);
>>  pci_register_bar(&d->pci, 0, 0, &d->mmio);
>>  if (d->msi) {
>> -msi_init(&d->pci, 0x50, 1, true, false);
>> +msi_init(&d->pci, 0x60, 1, true, false);
>>  }
>>
>>  hda_codec_bus_init(DEVICE(pci), &d->codecs, sizeof(d->codecs),
>>
> 
> Does this need a compat property?

Sigh, right, it's guest visible.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH for-2.1 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 09:56, Igor Mammedov ha scritto:
>> > It doesn't handle the case of ACPI tables that shrink, which can happen
>> > as well.  I guess if this ever happens we can just hard-code the table
>> > size of the "old" versions to something big enough (64K?) and keep using
>> > fine-grained sizing.
> That was intentionally omitted in here so far size only goes up from 1.7 to
> 2.1. My though was that can enforce minimum size later during 2.2 cycle
> Anyway, I'll think more about it, and maybe post additional patch
> on top of this to set minimum size if I find a reason for it to be in 2.1.

No, there's no need for it in 2.1.

Paolo



Re: [Qemu-devel] [PATCH for-2.1 v2 1/2] migration: load smaller RAMBlock to a bigger one if permitted

2014-07-28 Thread Laszlo Ersek
On 07/28/14 10:03, Igor Mammedov wrote:
> Add API to mark memory region as extend-able on migration,
> to allow migration code to load smaller RAMBlock into
> a bigger one on destination QEMU instance.
> 
> This will allow to fix broken migration from QEMU 1.7/2.0 to
> QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
> versions by marking ACPI tables ROM blob as extend-able.
> So that smaller tables from previous version could be always
> migrated to a bigger rom blob on new version.
> 
> Credits-for-idea: Michael S. Tsirkin 
> Signed-off-by: Igor Mammedov 
> ---
> v2:
>   fixed patch as suggested by Laszlo
> ---
>  arch_init.c | 22 +-
>  exec.c  |  8 
>  include/exec/memory.h   | 11 +++
>  include/exec/ram_addr.h |  3 +++
>  memory.c|  5 +
>  5 files changed, 44 insertions(+), 5 deletions(-)

Thank you.

Reviewed-by: Laszlo Ersek 




Re: [Qemu-devel] Possible null-ptr dereference

2014-07-28 Thread mateusz.krzywicki
Hey,

Yup, thanks, task closed ;-)

Best regards,
Mateusz Krzywicki

From: arei.gong...@huawei.com
To: mateusz.krzywi...@windowslive.com; qemu-devel@nongnu.org
CC: stefa...@redhat.com; kw...@redhat.com
Subject: RE: [Qemu-devel] Possible null-ptr dereference
Date: Mon, 28 Jul 2014 06:03:45 +









Hi,
 
Should be easy to fix though. Does the following help?
 
(Cc’ing Stefan & Kevin)
 
-->
xen_disk:  fix possible null-ptr dereference

 
Signed-off-by: Gonglei 
---
hw/block/xen_disk.c | 1 +
1
 file changed, 1 insertion(+)
 
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index aed5b5b..a221d0b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -589,6 +589,7 @@ static int blk_send_response_one(struct ioreq *ioreq)
 break;
 default:
 dst = NULL;
+return 0;
 }
 memcpy(dst, &resp, sizeof(resp));
 blkdev->rings.common.rsp_prod_pvt++;
--
 
Best regards,
-Gonglei

 



From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org 
[mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org]
On Behalf Of mateusz.krzywi...@windowslive.com

Sent: Saturday, July 26, 2014 6:52 PM

To: qemu-devel@nongnu.org

Subject: [Qemu-devel] Possible null-ptr dereference


 


Hey,


 


Found a little bug in latest qemu:


 


In function: 


static int blk_send_response_one(struct ioreq *ioreq)


 


File: 


qemu\hw\block\xen_disk.c


 


Code:


 



default:


dst = NULL;


}


memcpy(dst, &resp, sizeof(resp));



 


 


Just add simple check for dst and it will be all cool ;-)


 


Best regards,


Mateusz Krzywicki



  

Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gerd Hoffmann
  Hi,

> > ... because you can just copy the suffix from the old entry here,
> > instead of expecting the caller pass it in.
> > 
> Okay, agreed.
> 
> But we should also think about the situation which a device don't have
> old entry in global fw_boot_order list.

Throw an error?

I think it is ok to allow only *changing* the bootindex.

cheers,
  Gerd





Re: [Qemu-devel] [RFC PATCH v2 08/49] hpet: fixing saving and loading process

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> VM clock does not run while saving, so there is no need for saving the ticks
> in HPET. Also added saving of hpet_offset field.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/timer/hpet.c |   13 +
>  1 files changed, 1 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index e160e8f..73401b9 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -222,14 +222,6 @@ static void update_irq(struct HPETTimer *timer, int set)
>  }
>  }
>  
> -static void hpet_pre_save(void *opaque)
> -{
> -HPETState *s = opaque;
> -
> -/* save current counter value */
> -s->hpet_counter = hpet_get_ticks(s);
> -}
> -
>  static int hpet_pre_load(void *opaque)
>  {
>  HPETState *s = opaque;
> @@ -255,9 +247,6 @@ static int hpet_post_load(void *opaque, int version_id)
>  {
>  HPETState *s = opaque;
>  
> -/* Recalculate the offset between the main counter and guest time */
> -s->hpet_offset = ticks_to_ns(s->hpet_counter) - 
> qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -
>  /* Push number of timers into capability returned via HPET_ID */
>  s->capability &= ~HPET_ID_NUM_TIM_MASK;
>  s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
> @@ -308,13 +297,13 @@ static const VMStateDescription vmstate_hpet = {
>  .name = "hpet",
>  .version_id = 2,
>  .minimum_version_id = 1,
> -.pre_save = hpet_pre_save,
>  .pre_load = hpet_pre_load,
>  .post_load = hpet_post_load,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT64(config, HPETState),
>  VMSTATE_UINT64(isr, HPETState),
>  VMSTATE_UINT64(hpet_counter, HPETState),
> +VMSTATE_UINT64(hpet_offset, HPETState),

This needs a version bump.

Paolo

>  VMSTATE_UINT8_V(num_timers, HPETState, 2),
>  VMSTATE_VALIDATE("num_timers in range", hpet_validate_num_timers),
>  VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
> 
> 
> 




Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gonglei (Arei)
> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Monday, July 28, 2014 4:31 PM
> Subject: Re: [PATCH v2 1/7] bootindex: add modify_boot_device_path function
> 
>   Hi,
> 
> > > ... because you can just copy the suffix from the old entry here,
> > > instead of expecting the caller pass it in.
> > >
> > Okay, agreed.
> >
> > But we should also think about the situation which a device don't have
> > old entry in global fw_boot_order list.
> 
> Throw an error?
> 
No. I should firstly use the suffix which the caller pass in IMHO,
just like V3 I have posted, Thanks.

> I think it is ok to allow only *changing* the bootindex.
> 
Yes, that's no problem.

Best regards,
-Gonglei


Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

2014-07-28 Thread Igor Mammedov
On Mon, 28 Jul 2014 08:03:25 +
Igor Mammedov  wrote:

> It fixes migration failure for machine type pc-i440fx-1.7 from
> QEMU 1.7/2.0 to QEMU 2.1
> 
> Migration fails due to ACPI tables size grows across 1.7-2.1
> versions. That causes ACPI tables ROM blob to change its size
> differently for the same configurations on different QEMU versions.
> As result migration code bails out when attempting to load
> smaller ROM blob into a bigger one on a newer QEMU.
> To trigger failure it's enough to add pci-bridge device to QEMU CLI
> 
> Marking ACPI tables ROM blob as extend-able on migration allows
> QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
> forward migration failure introduced since 2.0 which affects
> only configurations that cause ACPI ROM blob size change.
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Laszlo Ersek 
Self-NACK

I'm sorry for mess, I've tested it on for i386 target,
but patch breaks build for other targets.
I'll resubmit v3 shortly.

> ---
>  hw/core/loader.c | 6 +-
>  hw/i386/acpi-build.c | 2 +-
>  include/hw/loader.h  | 5 +++--
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 2bf6b8f..d09b894 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -722,7 +722,8 @@ err:
>  
>  void *rom_add_blob(const char *name, const void *blob, size_t len,
> hwaddr addr, const char *fw_file_name,
> -   FWCfgReadCallback fw_callback, void *callback_opaque)
> +   FWCfgReadCallback fw_callback, void *callback_opaque,
> +   bool extendable)
>  {
>  Rom *rom;
>  void *data = NULL;
> @@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, 
> size_t len,
>  
>  if (rom_file_has_mr) {
>  data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
> +if (extendable) {
> +memory_region_permit_extendable_migration(rom->mr);
> +}
>  } else {
>  data = rom->data;
>  }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebc5f03..651c06b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState 
> *build_state, GArray *blob,
> const char *name)
>  {
>  return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
> -acpi_build_update, build_state);
> +acpi_build_update, build_state, true);
>  }
>  
>  static const VMStateDescription vmstate_acpi_build = {
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 796cbf9..e5a24ac 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
>   bool option_rom);
>  void *rom_add_blob(const char *name, const void *blob, size_t len,
> hwaddr addr, const char *fw_file_name,
> -   FWCfgReadCallback fw_callback, void *callback_opaque);
> +   FWCfgReadCallback fw_callback, void *callback_opaque,
> +   bool extendable);
>  int rom_add_elf_program(const char *name, void *data, size_t datasize,
>  size_t romsize, hwaddr addr);
>  int rom_load_all(void);
> @@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
>  #define rom_add_file_fixed(_f, _a, _i)  \
>  rom_add_file(_f, NULL, _a, _i, false)
>  #define rom_add_blob_fixed(_f, _b, _l, _a)  \
> -rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
> +rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
>  
>  #define PC_ROM_MIN_VGA 0xc
>  #define PC_ROM_MIN_OPTION  0xc8000




Re: [Qemu-devel] [RFC PATCH v2 07/49] kvmapic: fixing loading vmstate

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> vapic state should not be synchronized with APIC while loading,
> because APIC state could be not loaded yet at that moment.
> We just save vapic_paddr in APIC VMState instead of synchronization.

This comment is now obsolete:

include/hw/i386/apic_internal.h:hwaddr vapic_paddr; /* note: persistence 
via kvmvapic */

> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/i386/kvmvapic.c|   22 +-
>  hw/intc/apic_common.c |5 -
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index cb855c7..417ab6a 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -351,6 +351,24 @@ static int get_kpcr_number(X86CPU *cpu)
>  return kpcr.number;
>  }
>  
> +static int vapic_enable_post_load(VAPICROMState *s, X86CPU *cpu)
> +{
> +int cpu_number = get_kpcr_number(cpu);
> +hwaddr vapic_paddr;
> +static const uint8_t enabled = 1;
> +
> +if (cpu_number < 0) {
> +return -1;
> +}
> +vapic_paddr = s->vapic_paddr +
> +(((hwaddr)cpu_number) << VAPIC_CPU_SHIFT);
> +cpu_physical_memory_rw(vapic_paddr + offsetof(VAPICState, enabled),
> +   (void *)&enabled, sizeof(enabled), 1);
> +s->state = VAPIC_ACTIVE;
> +
> +return 0;
> +}
> +
>  static int vapic_enable(VAPICROMState *s, X86CPU *cpu)
>  {
>  int cpu_number = get_kpcr_number(cpu);
> @@ -731,7 +749,9 @@ static void do_vapic_enable(void *data)
>  VAPICROMState *s = data;
>  X86CPU *cpu = X86_CPU(first_cpu);
>  
> -vapic_enable(s, cpu);
> +/* Do not synchronize with APIC, because it was not loaded yet.
> +   Just call the enable function which does not have synchronization. */
> +vapic_enable_post_load(s, cpu);
>  }
>  
>  static int vapic_post_load(void *opaque, int version_id)
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index ce3d903..9d75ee0 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -347,7 +347,7 @@ static int apic_dispatch_post_load(void *opaque, int 
> version_id)
>  
>  static const VMStateDescription vmstate_apic_common = {
>  .name = "apic",
> -.version_id = 3,
> +.version_id = 4,
>  .minimum_version_id = 3,
>  .minimum_version_id_old = 1,
>  .load_state_old = apic_load_old,
> @@ -374,6 +374,9 @@ static const VMStateDescription vmstate_apic_common = {
>  VMSTATE_INT64(next_time, APICCommonState),
>  VMSTATE_INT64(timer_expiry,
>APICCommonState), /* open-coded timer state */
> +VMSTATE_INT32_V(sipi_vector, APICCommonState, 4),
> +VMSTATE_INT32_V(wait_for_sipi, APICCommonState, 4),

This could be a subsection.  sipi_vector is only used (needed) if wait_for_sipi 
!= 0.

> +VMSTATE_UINT64_V(vapic_paddr, APICCommonState, 4),

Here you could also use a subsection, where the "needed" function returns false 
if vapic_paddr == 0.

Paolo


>  VMSTATE_END_OF_LIST()
>  }
>  };
> 
> 
> 




Re: [Qemu-devel] [PATCH v12 0/6] qcow2, raw: add preallocation=full and preallocation=falloc

2014-07-28 Thread Hu Tao
ping...

All the 6 patches have reviewed-by now.

On Fri, Jul 11, 2014 at 02:09:57PM +0800, Hu Tao wrote:
> This series adds two preallocation mode to qcow2 and raw:
> 
> Option preallocation=full preallocates disk space for image by writing
> zeros to disk, this ensures disk space in any cases.
> 
> Option preallocation=falloc preallocates disk space by calling
> posix_fallocate(). This is faster than preallocation=full.
> 
> This series depends on patches 1-3 of Max's series 'qemu-img: Implement
> commit like QMP'. Specifically, patch 6 'qcow2: Add falloc and full
> preallocation option' uses minimal_blob_size() introduced by Max's patch
> 'qcow2: Optimize bdrv_make_empty()'.
> 
> The series is also at https://github.com/taohu/qemu/commits/preallocation-v12
> for you to check out, including depended patches from Max.
> 
> Eric, I'm afraid now we missed qemu 2.1, so patch 1 is still sent
> with this series.
> 
> changes to v11:
> 
>  - fix test case 049 (patch 4)
>  - unsigned nl2e -> uint64_t nl2e (patch 6)
>  - use >> instead of / (patch 6)
> 
> 
> Hu Tao (6):
>   block: round up file size to nearest sector
>   raw, qcow2: don't convert file size to sector size
>   rename parse_enum_option to qapi_enum_parse and make it public
>   qapi: introduce PreallocMode and a new PreallocMode full.
>   raw-posix: Add falloc and full preallocation option
>   qcow2: Add falloc and full preallocation option
> 
>  block/qcow2.c  | 56 +---
>  block/raw-posix.c  | 92 
> +++---
>  block/raw-win32.c  |  6 +--
>  blockdev.c | 30 +++
>  include/qapi/util.h| 17 +
>  qapi/Makefile.objs |  1 +
>  qapi/block-core.json   | 17 +
>  qapi/qapi-util.c   | 32 
>  tests/qemu-iotests/049.out |  2 +-
>  tests/qemu-iotests/082.out | 54 +--
>  tests/qemu-iotests/096 | 64 
>  tests/qemu-iotests/096.out | 14 +++
>  tests/qemu-iotests/group   |  1 +
>  13 files changed, 296 insertions(+), 90 deletions(-)
>  create mode 100644 include/qapi/util.h
>  create mode 100644 qapi/qapi-util.c
>  create mode 100755 tests/qemu-iotests/096
>  create mode 100644 tests/qemu-iotests/096.out
> 
> -- 
> 1.9.3
> 



[Qemu-devel] [PATCH for-2.1 v3 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

2014-07-28 Thread Igor Mammedov
It fixes migration failure for machine type pc-i440fx-1.7 from
QEMU 1.7/2.0 to QEMU 2.1

Migration fails due to ACPI tables size grows across 1.7-2.1
versions. That causes ACPI tables ROM blob to change its size
differently for the same configurations on different QEMU versions.
As result migration code bails out when attempting to load
smaller ROM blob into a bigger one on a newer QEMU.
To trigger failure it's enough to add pci-bridge device to QEMU CLI

Marking ACPI tables ROM blob as extend-able on migration allows
QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
forward migration failure introduced since 2.0 which affects
only configurations that cause ACPI ROM blob size change.

Signed-off-by: Igor Mammedov 
Reviewed-by: Laszlo Ersek 
---
v3:
  fix build breakage of lm32 target
---
 hw/core/loader.c   | 6 +-
 hw/i386/acpi-build.c   | 2 +-
 hw/lm32/lm32_hwsetup.h | 2 +-
 include/hw/loader.h| 5 +++--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..d09b894 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -722,7 +722,8 @@ err:
 
 void *rom_add_blob(const char *name, const void *blob, size_t len,
hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque)
+   FWCfgReadCallback fw_callback, void *callback_opaque,
+   bool extendable)
 {
 Rom *rom;
 void *data = NULL;
@@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, 
size_t len,
 
 if (rom_file_has_mr) {
 data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+if (extendable) {
+memory_region_permit_extendable_migration(rom->mr);
+}
 } else {
 data = rom->data;
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..651c06b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState 
*build_state, GArray *blob,
const char *name)
 {
 return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
-acpi_build_update, build_state);
+acpi_build_update, build_state, true);
 }
 
 static const VMStateDescription vmstate_acpi_build = {
diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h
index 9fd5e69..943130c 100644
--- a/hw/lm32/lm32_hwsetup.h
+++ b/hw/lm32/lm32_hwsetup.h
@@ -73,7 +73,7 @@ static inline void hwsetup_free(HWSetup *hw)
 static inline void hwsetup_create_rom(HWSetup *hw,
 hwaddr base)
 {
-rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE, base, NULL, NULL, 
NULL);
+rom_add_blob_fixed("hwsetup", hw->data, TARGET_PAGE_SIZE, base);
 }
 
 static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..e5a24ac 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
  bool option_rom);
 void *rom_add_blob(const char *name, const void *blob, size_t len,
hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque);
+   FWCfgReadCallback fw_callback, void *callback_opaque,
+   bool extendable);
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
 size_t romsize, hwaddr addr);
 int rom_load_all(void);
@@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)  \
 rom_add_file(_f, NULL, _a, _i, false)
 #define rom_add_blob_fixed(_f, _b, _l, _a)  \
-rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
+rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
 
 #define PC_ROM_MIN_VGA 0xc
 #define PC_ROM_MIN_OPTION  0xc8000
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 2/7] tests: Add virtio device initialization

2014-07-28 Thread Stefan Hajnoczi
On Fri, Jul 25, 2014 at 07:01:47PM +0200, Marc Marí wrote:
> > > @@ -73,3 +97,11 @@ QVirtioPCIDevice
> > > *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type) 
> > >  return dev;
> > >  }
> > > +
> > > +void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
> > > +{
> > > +qpci_device_enable(d->pdev);
> > > +d->addr = qpci_iomap(d->pdev, 0);
> > > +g_assert(d->addr != NULL);
> > > +}
> > 
> > Where is qpci_iounmap() called to clean up?
> 
> Missed. Also, it is unimplemented.

It would be much harder to add in the appropriate guest_free() calls
later so users should still call it.  Just like we should call
guest_free() even if it is currently unimplemented.

Stefan


pgpw3RjuSra6Y.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gerd Hoffmann
> > I think it is ok to allow only *changing* the bootindex.
> > 
> Yes, that's no problem.

But then yoy always  will have a old entry where you can take the suffix
from, and you don't need the suffix as parameter for the monitor
command.

cheers,
  Gerd





Re: [Qemu-devel] [RFC PATCH v2 09/49] pckbd: adding new fields to vmstate

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> This patch adds outport to VMState to allow correct saving and restoring
> the state of PC keyboard controller.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/input/pckbd.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index ca1cffc..19f6658 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -371,13 +371,14 @@ static void kbd_reset(void *opaque)
>  
>  static const VMStateDescription vmstate_kbd = {
>  .name = "pckbd",
> -.version_id = 3,
> +.version_id = 4,
>  .minimum_version_id = 3,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT8(write_cmd, KBDState),
>  VMSTATE_UINT8(status, KBDState),
>  VMSTATE_UINT8(mode, KBDState),
>  VMSTATE_UINT8(pending, KBDState),
> +VMSTATE_UINT8_V(outport, KBDState, 4),
>  VMSTATE_END_OF_LIST()
>  }
>  };
> 
> 
> 

Again it would be nice to use a subsection.  You can use as the
"default" value

KBD_OUT_RESET | KBD_OUT_A20
 | (kbd->status & KBD_STAT_OBF ? KBD_OUT_OBF : 0)
 | (kbd->status & KBD_STAT_MOUSE_OBF ? KBD_OUT_MOUSE_OBF : 0)

If the value of outport matches this, you need not write it.  It's not
trivial, but you could do it like this:

- needed: return false if outport doesn't have the value above

- subsection post_load: set kbd->outport_present = 1

- device post_load: reconstruct outport if kbd->outport_present == 0

Paolo



Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gonglei (Arei)
Hi,

> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Monday, July 28, 2014 5:28 PM
> Subject: Re: [PATCH v2 1/7] bootindex: add modify_boot_device_path function
> 
> > > I think it is ok to allow only *changing* the bootindex.
> > >
> > Yes, that's no problem.
> 
> But then yoy always  will have a old entry where you can take the suffix
> from, and you don't need the suffix as parameter for the monitor
> command.
> 
No, optional. 

Because the bootindex property is not a necessary property for devices.
If a device, such as IDE cdrom haven't attach the bootindex in qemu booting 
command line, the global fw_boot_order will not have its entry. So, we should
save the suffix as parameter.

Best regards,
-Gonglei




Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> This patch adds virtual clock-dependent timers to VMState to allow correct
> saving and restoring the state of RTL8139 network controller.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/net/rtl8139.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 90bc5ec..992caf0 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -3289,7 +3289,7 @@ static void rtl8139_pre_save(void *opaque)
>  
>  static const VMStateDescription vmstate_rtl8139 = {
>  .name = "rtl8139",
> -.version_id = 4,
> +.version_id = 5,
>  .minimum_version_id = 3,
>  .post_load = rtl8139_post_load,
>  .pre_save  = rtl8139_pre_save,
> @@ -3363,6 +3363,9 @@ static const VMStateDescription vmstate_rtl8139 = {
>  VMSTATE_STRUCT(tally_counters, RTL8139State, 0,
> vmstate_tally_counters, RTL8139TallyCounters),
>  
> +VMSTATE_TIMER_V(timer, RTL8139State, 5),

timer need not be migrated, because it is reinstated by rtl8139_post_load.

> +VMSTATE_INT64_V(TimerExpire, RTL8139State, 5),

This can be in a subsection, migrated only if non-zero.

Paolo

>  VMSTATE_UINT32_V(cplus_enabled, RTL8139State, 4),
>  VMSTATE_END_OF_LIST()
>  },
> 
> 
> 




Re: [Qemu-devel] [RFC PATCH v2 12/49] mc146818rtc: add missed field to vmstate

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:03, Pavel Dovgalyuk ha scritto:
> This patch adds irq_reinject_on_ack_count field to VMState to allow correct
> saving/loading the state of MC146818 RTC.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/timer/mc146818rtc.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 9d817ca..c204abb 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -735,7 +735,7 @@ static int rtc_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_rtc = {
>  .name = "mc146818rtc",
> -.version_id = 3,
> +.version_id = 4,
>  .minimum_version_id = 1,
>  .post_load = rtc_post_load,
>  .fields = (VMStateField[]) {
> @@ -752,6 +752,7 @@ static const VMStateDescription vmstate_rtc = {
>  VMSTATE_INT64_V(offset, RTCState, 3),
>  VMSTATE_TIMER_V(update_timer, RTCState, 3),
>  VMSTATE_UINT64_V(next_alarm_time, RTCState, 3),
> +VMSTATE_UINT16_V(irq_reinject_on_ack_count, RTCState, 4),

Also can be a subsection, migrated only if nonzero.

Paolo

>  VMSTATE_END_OF_LIST()
>  }
>  };
> 
> 
> 




Re: [Qemu-devel] [RFC PATCH v2 04/49] fdc: adding vmstate for save/restore

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> VMState added by this patch preserves correct
> loading of the FDC device state.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  hw/block/fdc.c |   11 +--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 490d127..132310a 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -697,12 +697,17 @@ static const VMStateDescription 
> vmstate_fdrive_media_rate = {
>  
>  static const VMStateDescription vmstate_fdrive = {
>  .name = "fdrive",
> -.version_id = 1,
> +.version_id = 2,
>  .minimum_version_id = 1,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT8(head, FDrive),
>  VMSTATE_UINT8(track, FDrive),
>  VMSTATE_UINT8(sect, FDrive),
> +VMSTATE_UINT8_V(last_sect, FDrive, 2),
> +VMSTATE_UINT8_V(max_track, FDrive, 2),
> +VMSTATE_UINT16_V(bps, FDrive, 2),
> +VMSTATE_UINT8_V(ro, FDrive, 2),
> +VMSTATE_UINT8_V(perpendicular, FDrive, 2),

Perpendicular can be added to a subsection, migrated only if nonzero.
The others can be reconstructed by calling fd_revalidate in
vmstate_fdrive's post_load callback.


>  VMSTATE_END_OF_LIST()
>  },
>  .subsections = (VMStateSubsection[]) {
> @@ -736,7 +741,7 @@ static int fdc_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_fdc = {
>  .name = "fdc",
> -.version_id = 2,
> +.version_id = 3,
>  .minimum_version_id = 2,
>  .pre_save = fdc_pre_save,
>  .post_load = fdc_post_load,
> @@ -769,6 +774,8 @@ static const VMStateDescription vmstate_fdc = {
>  VMSTATE_UINT8_EQUAL(num_floppies, FDCtrl),
>  VMSTATE_STRUCT_ARRAY(drives, FDCtrl, MAX_FD, 1,
>   vmstate_fdrive, FDrive),
> +VMSTATE_INT32_V(reset_sensei, FDCtrl, 3),

Subsection, only migrated if nonzero.

> +VMSTATE_TIMER_V(result_timer, FDCtrl, 3),

Subsection, only migrated if pending.

Paolo

>  VMSTATE_END_OF_LIST()
>  }
>  };
> 
> 
> 




Re: [Qemu-devel] Possible null-ptr dereference

2014-07-28 Thread Stefan Hajnoczi
On Mon, Jul 28, 2014 at 06:03:45AM +, Gonglei (Arei) wrote:
> Hi,
> 
> Should be easy to fix though. Does the following help?
> 
> (Cc'ing Stefan & Kevin)
> 
> -->
> xen_disk:  fix possible null-ptr dereference
> 
> Signed-off-by: Gonglei 
> ---
> hw/block/xen_disk.c | 1 +
> 1 file changed, 1 insertion(+)

This code path can never be reached since protocol is always set to one
of 3 valid values in xen_disk.c.  Therefore, I'm not merging this for
QEMU 2.1 where we are only taking critical bug fixes now.

Still, it will help silence static checkers and make the intent clear to
readers.

Thanks, applied to my block-next tree for QEMU 2.2:
https://github.com/stefanha/qemu/commits/block-next

Stefan


pgpu0OfPHR42l.pgp
Description: PGP signature


Re: [Qemu-devel] Possible null-ptr dereference

2014-07-28 Thread Gonglei (Arei)
> -Original Message-
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> Sent: Monday, July 28, 2014 5:49 PM
> To: Gonglei (Arei)
> Cc: mateusz.krzywi...@windowslive.com; qemu-devel@nongnu.org;
> kw...@redhat.com
> Subject: Re: [Qemu-devel] Possible null-ptr dereference
> 
> On Mon, Jul 28, 2014 at 06:03:45AM +, Gonglei (Arei) wrote:
> > Hi,
> >
> > Should be easy to fix though. Does the following help?
> >
> > (Cc'ing Stefan & Kevin)
> >
> > -->
> > xen_disk:  fix possible null-ptr dereference
> >
> > Signed-off-by: Gonglei 
> > ---
> > hw/block/xen_disk.c | 1 +
> > 1 file changed, 1 insertion(+)
> 
> This code path can never be reached since protocol is always set to one
> of 3 valid values in xen_disk.c.  Therefore, I'm not merging this for
> QEMU 2.1 where we are only taking critical bug fixes now.
> 
OK.

> Still, it will help silence static checkers and make the intent clear to
> readers.
> 
> Thanks, applied to my block-next tree for QEMU 2.2:
> https://github.com/stefanha/qemu/commits/block-next
> 
Thanks.

Best regards,
-Gonglei





Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-28 Thread Pavel Dovgaluk
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> > This patch adds virtual clock-dependent timers to VMState to allow correct
> > saving and restoring the state of RTL8139 network controller.
> >
> > Signed-off-by: Pavel Dovgalyuk 
> > ---
> >  hw/net/rtl8139.c |5 -
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index 90bc5ec..992caf0 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -3289,7 +3289,7 @@ static void rtl8139_pre_save(void *opaque)
> >
> >  static const VMStateDescription vmstate_rtl8139 = {
> >  .name = "rtl8139",
> > -.version_id = 4,
> > +.version_id = 5,
> >  .minimum_version_id = 3,
> >  .post_load = rtl8139_post_load,
> >  .pre_save  = rtl8139_pre_save,
> > @@ -3363,6 +3363,9 @@ static const VMStateDescription vmstate_rtl8139 = {
> >  VMSTATE_STRUCT(tally_counters, RTL8139State, 0,
> > vmstate_tally_counters, RTL8139TallyCounters),
> >
> > +VMSTATE_TIMER_V(timer, RTL8139State, 5),
> 
> timer need not be migrated, because it is reinstated by rtl8139_post_load.
> 

  That's true for normal execution.
  In replay execution mode post_load can be called before cached virtual clock
values are loaded. This may cause invalid setting of the timer and raising
an IRQ which didn't happen in record mode.
  I will update this patch to fix post_load function and avoid this 
non-deterministic behavior.

Pavel Dovgalyuk




Re: [Qemu-devel] [RFC PATCH v2 06/49] serial: fixing vmstate for save/restore

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> -.version_id = 3,
> +.version_id = 4,
>  .minimum_version_id = 2,
>  .pre_save = serial_pre_save,
>  .post_load = serial_post_load,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT16_V(divider, SerialState, 2),
>  VMSTATE_UINT8(rbr, SerialState),
> +VMSTATE_UINT8_V(thr, SerialState, 4),
> +VMSTATE_UINT8_V(tsr, SerialState, 4),
>  VMSTATE_UINT8(ier, SerialState),
>  VMSTATE_UINT8(iir, SerialState),
>  VMSTATE_UINT8(lcr, SerialState),
> @@ -613,6 +627,15 @@ const VMStateDescription vmstate_serial = {
>  VMSTATE_UINT8(msr, SerialState),
>  VMSTATE_UINT8(scr, SerialState),
>  VMSTATE_UINT8_V(fcr_vmstate, SerialState, 3),
> +VMSTATE_INT32_V(thr_ipending, SerialState, 4),

Subsection, only migrated if it doesn't match "(s->iir & UART_IIR_ID) ==
UART_IIR_THRI".

> +VMSTATE_INT32_V(last_break_enable, SerialState, 4),

Can be reconstructed in the post_load callback from s->lcr.

> +VMSTATE_INT32_V(tsr_retry, SerialState, 4),

Subsection, only migrated if nonzero.  thr/tsr can be in this subsection
as well.

> +VMSTATE_STRUCT(recv_fifo, SerialState, 4, vmstate_fifo8, Fifo8),
> +VMSTATE_STRUCT(xmit_fifo, SerialState, 4, vmstate_fifo8, Fifo8),

Two subsections, only transmitted if nonempty.

> +VMSTATE_TIMER_V(fifo_timeout_timer, SerialState, 4),

Subsection, only transmitted if pending.

> +VMSTATE_INT32_V(timeout_ipending, SerialState, 4),

Subsection, transmitted only if nonzero.

> +VMSTATE_INT32_V(poll_msl, SerialState, 4),
> +VMSTATE_TIMER_V(modem_status_poll, SerialState, 4),

Both in a subsection, only migrated if poll_msl is not -1.

Paolo

>  VMSTATE_END_OF_LIST()




Re: [Qemu-devel] Possible null-ptr dereference

2014-07-28 Thread Peter Maydell
On 28 July 2014 10:49, Stefan Hajnoczi  wrote:
> On Mon, Jul 28, 2014 at 06:03:45AM +, Gonglei (Arei) wrote:
>> Hi,
>>
>> Should be easy to fix though. Does the following help?
>>
>> (Cc'ing Stefan & Kevin)
>>
>> -->
>> xen_disk:  fix possible null-ptr dereference
>>
>> Signed-off-by: Gonglei 
>> ---
>> hw/block/xen_disk.c | 1 +
>> 1 file changed, 1 insertion(+)
>
> This code path can never be reached since protocol is always set to one
> of 3 valid values in xen_disk.c.

Maybe g_assert_not_reached(); ?

thanks
-- PMM



Re: [Qemu-devel] [RFC PATCH v2 05/49] parallel: adding vmstate for save/restore

2014-07-28 Thread Paolo Bonzini
Il 17/07/2014 13:02, Pavel Dovgalyuk ha scritto:
> +.fields  = (VMStateField []) {
> +VMSTATE_UINT8(state.dataw, ISAParallelState),
> +VMSTATE_UINT8(state.datar, ISAParallelState),
> +VMSTATE_UINT8(state.status, ISAParallelState),
> +VMSTATE_UINT8(state.control, ISAParallelState),
> +VMSTATE_INT32(state.irq_pending, ISAParallelState),
> +VMSTATE_INT32(state.hw_driver, ISAParallelState),

Static, doesn't need migration.

> +VMSTATE_INT32(state.epp_timeout, ISAParallelState),
> +VMSTATE_INT32(state.it_shift, ISAParallelState),

Static, doesn't need migration.

> +VMSTATE_END_OF_LIST()




Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gerd Hoffmann
  Hi,

> > > > I think it is ok to allow only *changing* the bootindex.
> > > >
> > > Yes, that's no problem.
> > 
> > But then yoy always  will have a old entry where you can take the suffix
> > from, and you don't need the suffix as parameter for the monitor
> > command.
> > 
> No, optional. 

> Because the bootindex property is not a necessary property for devices.
> If a device, such as IDE cdrom haven't attach the bootindex in qemu booting 
> command line, the global fw_boot_order will not have its entry.

I'd suggest to simply not support this and throw an error then.

> So, we should
> save the suffix as parameter.

I think it is a bad idea to have the suffix as monitor command
parameter.  It is a implementation detail which the monitor users should
not have to worry about.

Easiest way to do this is to allow *changing* an existing bootindex
entry only, and not support *adding* new boot entries.  The user has to
set a bootindex for any device it might want to boot from in the future
then.  I think this is acceptable.

If you want support adding bootentries at runtime (and it probably makes
sense to support removing entries too in that case) the suffix handling
should be reworked.  The suffix could be stored in DeviceState instead
of being passed to the add_bootentry function, so you can add boot
entries later on without expecting the user to know what the correct
suffix is.

cheers,
  Gerd





Re: [Qemu-devel] [RFC PATCH v2 10/49] rtl8139: adding new fields to vmstate

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 11:54, Pavel Dovgaluk ha scritto:
>>> > > +VMSTATE_TIMER_V(timer, RTL8139State, 5),
>> > 
>> > timer need not be migrated, because it is reinstated by rtl8139_post_load.
>> > 
>   That's true for normal execution.
>   In replay execution mode post_load can be called before cached virtual clock
> values are loaded. This may cause invalid setting of the timer and raising
> an IRQ which didn't happen in record mode.
>   I will update this patch to fix post_load function and avoid this 
> non-deterministic behavior.

This is what worries me of this series.  These invariants are not
documented anywhere, and people will break them unless you add
assertions that also hold in normal mode.

Paolo



Re: [Qemu-devel] [RFC PATCH v2 00/49] Series short description

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 09:50, Pavel Dovgaluk ha scritto:
>> - patches 2 to 13 probably should try to use subsections, so that VMs
>> that do not use the devices try not to save the extra data and keep
>> backwards migration compatibility (at least try to)
> 
>  Could you give me and example?
>  As I know, subsection is loaded when some predicate function returns true. 
> How can I 
> construct such a function for integratorcp module? What kind of condition 
> should it check?
>  In this module I just added missed vmstates (it does not saved/restored at 
> all
> by the master version).

I tried to review the x86 ones.  These are the most interesting ones,
because it's the only board where we support cross-version migration.

Parallel is (quite obviously) unsalvageable, and I think HPET is too.

>> - patch 16 should also use subsections, and perhaps apply to all other
>> CPUs too?
> 
>  We implemented replay only for i386 and ARM. If we'll change other targets, 
> it will not
> add record/replay capabilities to them, but can confuse the reviewers.

Why are these fields only required by record/replay?

>> - patches 23-24-25 perhaps could try using icount, like Konrad's patch do?
> 
>  Using faster icount (like in Konrad's patches) is the our next aim. It 
> obviously will 
> increase the speed of recording process. But now I submitted slower, but more 
> conservative
> version of icount which we had already tested.

I see.

>> - patch 27 makes sense but VIRTUAL is used to skip blinking when the VM
>> is stopped
> 
>  Right, this is kind of hack. I haven't found better solution yet.

I think it's okay to use REALTIME, just add runstate_is_running() to the
"if (now >= s->cursor_blink_time)".

That said, "every read to virtual clock is written to the
replay log" worries me a bit from the point of view of thread-safety.
Do you need to log all reads because you don't use icount?  Reads can
only happen at given points if you use icount, and you could simply log
the icount value at each synchronization point.

Paolo



Re: [Qemu-devel] [PATCH v2 1/7] bootindex: add modify_boot_device_path function

2014-07-28 Thread Gonglei (Arei)
> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Monday, July 28, 2014 6:02 PM
> Subject: Re: [PATCH v2 1/7] bootindex: add modify_boot_device_path function
> 
>   Hi,
> 
> > > > > I think it is ok to allow only *changing* the bootindex.
> > > > >
> > > > Yes, that's no problem.
> > >
> > > But then yoy always  will have a old entry where you can take the suffix
> > > from, and you don't need the suffix as parameter for the monitor
> > > command.
> > >
> > No, optional.
> 
> > Because the bootindex property is not a necessary property for devices.
> > If a device, such as IDE cdrom haven't attach the bootindex in qemu booting
> > command line, the global fw_boot_order will not have its entry.
> 
> I'd suggest to simply not support this and throw an error then.
> 
Ok.

> > So, we should
> > save the suffix as parameter.
> 
> I think it is a bad idea to have the suffix as monitor command
> parameter.  It is a implementation detail which the monitor users should
> not have to worry about.
> 
Yes. Actually I also have this misgivings.

> Easiest way to do this is to allow *changing* an existing bootindex
> entry only, and not support *adding* new boot entries.  The user has to
> set a bootindex for any device it might want to boot from in the future
> then.  I think this is acceptable.
> 
Hmm..

> If you want support adding bootentries at runtime (and it probably makes
> sense to support removing entries too in that case) the suffix handling
> should be reworked.  The suffix could be stored in DeviceState instead
> of being passed to the add_bootentry function, so you can add boot
> entries later on without expecting the user to know what the correct
> suffix is.
> 
That's a good idea! I think this is a good improvement. 

Any other comments? Thanks!

Best regards,
-Gonglei


Re: [Qemu-devel] [PATCH v2 7/7] libqos: Added basic virtqueue support to virtio implementation

2014-07-28 Thread Marc Marí
El Fri, 25 Jul 2014 23:37:43 +0200
Marc Marí  escribió:
> +
> +data = g_malloc0(512);
> +memread(r_req+16, data, 512);
> +g_assert_cmpstr(data, ==, "TEST");
> +g_free(data);
> +

guest_free() for both requests should be added here. Will be added for
v3

> +/* End test */
> +guest_free(alloc, vq->desc);
>  qvirtio_pci_disable_device(dev);
>  g_free(dev);
>  test_end();




Re: [Qemu-devel] [PULL for-2.1] Trivial patch for 2014-07-26

2014-07-28 Thread Peter Maydell
On 26 July 2014 08:18, Michael Tokarev  wrote:
> There's just one trivial patch this time, fixing another
> occurence of "allows to" in help text.  Please consider
> applying for 2.1.
>
> Thanks,
>
> /mjt
>
> The following changes since commit c60a57ff497667780132a3fcdc1500c83af5d5c0:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2014-07-25 16:58:41 +0100)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/trivial-patches-2014-07-26
>
> for you to fetch changes up to 2f47b403bd494b6717ef14c0d278d4b2d364b382:
>
>   qemu-options: fix another allows-to for -net l2tpv3 (2014-07-26 11:16:44 
> +0400)
>
> 
> trivial patches for 2014-07-26
>
> 
> Michael Tokarev (1):
>   qemu-options: fix another allows-to for -net l2tpv3
>
>  qemu-options.hx |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH v2 7/7] spapr: fix possible memory leak

2014-07-28 Thread Alexander Graf


On 25.07.14 08:52, arei.gong...@huawei.com wrote:

From: Gonglei 

get_boot_devices_list() will malloc memory, spapr_finalize_fdt
doesn't free it.

Signed-off-by: Chenliang 
Signed-off-by: Gonglei 


Thanks, applied to ppc-next-2.2.


Alex




Re: [Qemu-devel] [PATCH v3 0/6] spapr: rework memory nodes

2014-07-28 Thread Alexander Graf


On 21.07.14 05:15, Alexey Kardashevskiy wrote:

On 07/03/2014 01:10 PM, Alexey Kardashevskiy wrote:

c4177479 "spapr: make sure RMA is in first mode of first memory node"
introduced regression which prevents from running guests with memoryless
NUMA node#0 which may happen on real POWER8 boxes and which would make
sense to debug in QEMU.

This patchset aim is to fix that and also fix various code problems in
memory nodes generation.

These 2 patches could be merged (the resulting patch looks rather ugly):
spapr: Use DT memory node rendering helper for other nodes
spapr: Move DT memory node rendering to a helper


Alex, there are "numa: enable sparse node numbering ..." patches from Nish,
which set can go first so the other could rebase on top of it? Thanks!


Ping. This is for 2.2 indeed.


Thanks, applied all to ppc-next-2.2.


Alex




Re: [Qemu-devel] [PATCH 0/2] spapr: Move rtas and device-tree higher

2014-07-28 Thread Alexander Graf


On 21.07.14 05:02, Alexey Kardashevskiy wrote:

At the moment RTAS and device tree are located at 256MB max
which leaves too little space for huge initramdisk images and kernels.
This relaxes the limitation.

This is checkpatch.pl'ed version of:
[PATCH 1/2] loader: Add load_image_size() to replace load_image()
[PATCH 2/2] ppc/spapr: Locate RTAS and device-tree based on real RMA

Please comment. Thanks.


Thanks, applied all to ppc-next-2.2.


Alex




[Qemu-devel] [PATCH] target-mips: Ignore unassigned accesses with KVM

2014-07-28 Thread James Hogan
MIPS registers an unassigned access handler which raises a guest bus
error exception. However this causes QEMU to crash when KVM is enabled
as it isn't called from the main execution loop so longjmp() gets called
without a corresponding setjmp().

Until the KVM API can be updated to trigger a guest exception in
response to an MMIO exit, prevent the bus error exception being raised
from mips_cpu_unassigned_access() if KVM is enabled.

The check is at run time since the do_unassigned_access callback is
initialised before it is known whether KVM will be enabled.

The problem can be triggered with Malta emulation by making the guest
write to the reset region at physical address 0x1bf0, since it is
marked read-only which is treated as unassigned for writes.

Signed-off-by: James Hogan 
Cc: Aurelien Jarno 
Cc: Peter Maydell 
Cc: Paolo Bonzini 
Cc: Gleb Natapov 
Cc: Christoffer Dall 
Cc: Sanjay Lal 
---
 target-mips/op_helper.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 27651a4a00c1..df97b35f8701 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
+#include "sysemu/kvm.h"
 
 #ifndef CONFIG_USER_ONLY
 static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
@@ -2168,6 +2169,16 @@ void mips_cpu_unassigned_access(CPUState *cs, hwaddr 
addr,
 MIPSCPU *cpu = MIPS_CPU(cs);
 CPUMIPSState *env = &cpu->env;
 
+/*
+ * Raising an exception with KVM enabled will crash because it won't be 
from
+ * the main execution loop so the longjmp won't have a matching setjmp.
+ * Until we can trigger a bus error exception through KVM lets just ignore
+ * the access.
+ */
+if (kvm_enabled()) {
+return;
+}
+
 if (is_exec) {
 helper_raise_exception(env, EXCP_IBE);
 } else {
-- 
1.8.5.5




[Qemu-devel] [PATCH] pty: Fix byte loss bug when connecting to pty

2014-07-28 Thread Sebastian Tanase
When trying to print data to the pty, we first check if it is connected.
If not, we try to reconnect, but we drop the pending data even if we
have successfully reconnected; this makes us lose the first byte of the very
first transmission.
This small fix addresses the issue by checking once more if the pty is connected
after having tried to reconnect.

Signed-off-by: Sebastian Tanase 
---

To reproduce the bug, launch a qemu image that has a parallel port (say lp0)
and redirect it to a pty (-parallel pty). After the VM is launched,
open the corresponding pty on your host (cat /dev/pts/X) and send some
data from the VM to the host: echo "abcd" > /dev/lp0
The first time, the received string will be "bcd" instead of "abcd".
This bug can have important consequences if you try, for example,
to send a postscript file from a printer within the VM. Losing the
first character will render the .ps file unusable.
---
 qemu-char.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index 7acc03f..ce52d0f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1160,7 +1160,9 @@ static int pty_chr_write(CharDriverState *chr, const 
uint8_t *buf, int len)
 if (!s->connected) {
 /* guest sends data, check for (re-)connect */
 pty_chr_update_read_handler_locked(chr);
-return 0;
+if (!s->connected) {
+return 0;
+}
 }
 return io_channel_send(s->fd, buf, len);
 }
-- 
2.0.0.rc2




Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Michael S. Tsirkin
On Thu, Jul 24, 2014 at 04:32:09PM +0200, Paolo Bonzini wrote:
> Changing the ACPI table size causes migration to break, and the memory
> hotplug work opened our eyes on how horribly we were breaking things in
> 2.0 already.
> 
> The ACPI table size is rounded to the next 4k, which one would think
> gives some headroom.  In practice this is not the case, because the user
> can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
> 8 to the MADT) and so some "-smp" values will break the 4k boundary and
> fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.
> 
> To fix this, hard-code 64k as the maximum ACPI table size, which
> (despite being an order of magnitude smaller than 640k) should be enough
> for everyone.
> 
> To fix migration from QEMU 2.0, compute the payload size of QEMU 2.0
> and always use that one.  The previous patch shrunk the ACPI tables
> enough that the QEMU 2.0 size should always be enough.
> 
> Migration from QEMU 1.7 should work for guests that have a number of CPUs
> other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
> broken from QEMU 1.7 to QEMU 2.0 in the same way, though.
> 
> Even with this patch, QEMU 1.7 and 2.0 have two different ideas of
> "-M pc-i440fx-2.0" when there are PCI bridges.  Igor sent a patch to
> adopt the QEMU 1.7 definition.  I think distributions should apply
> it if they move directly from QEMU 1.7 to 2.1+ without ever packaging
> version 2.0.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>   replace magic constants with #defines [Igor]
>   remove stray line from comment [Laszlo]
> 
>  hw/i386/acpi-build.c | 71 
> +---
>  hw/i386/pc_piix.c| 19 ++
>  hw/i386/pc_q35.c |  5 
>  include/hw/i386/pc.h |  1 +
>  4 files changed, 92 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebc5f03..26d8dfa 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -25,7 +25,9 @@
>  #include 
>  #include "qemu-common.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/osdep.h"
>  #include "qemu/range.h"
> +#include "qemu/error-report.h"
>  #include "hw/pci/pci.h"
>  #include "qom/cpu.h"
>  #include "hw/i386/pc.h"
> @@ -52,6 +54,16 @@
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
>  
> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> + * -M pc-i440fx-2.0.

Let's just say 2.0 and earlier?

>  Even if the actual amount of AML generated grows
> + * a little bit, there should be plenty of free space since the DSDT
> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> + */
> +#define ACPI_BUILD_CPU_AML_SIZE97
> +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875

Let's put _LEGACY_ somewhere here?

> +
> +#define ACPI_BUILD_TABLE_SIZE  0x1
> +
>  typedef struct AcpiCpuInfo {
>  DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>  } AcpiCpuInfo;
> @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState {
>  struct AcpiBuildPciBusHotplugState *parent;
>  } AcpiBuildPciBusHotplugState;
>  
> +unsigned bsel_alloc;
> +

Patch will be better contained if instead of using a global
bsel_alloc, we actually go and count the devices that
have ACPI_PCIHP_PROP_BSEL.
You can just scan all devices, or all pci devices, it
should not matter.
This way, this code will be local to the legacy path.


>  static void acpi_get_dsdt(AcpiMiscInfo *info)
>  {
>  uint16_t *applesmc_sta;
> @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
>  static void acpi_set_pci_info(void)
>  {
>  PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
> -unsigned bsel_alloc = 0;
>  
> +assert(bsel_alloc == 0);
>  if (bus) {
>  /* Scan all PCI buses. Set property to enable acpi based hotplug. */
>  pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
> @@ -1440,13 +1454,14 @@ static
>  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>  {
>  GArray *table_offsets;
> -unsigned facs, dsdt, rsdt;
> +unsigned facs, ssdt, dsdt, rsdt;
>  AcpiCpuInfo cpu;
>  AcpiPmInfo pm;
>  AcpiMiscInfo misc;
>  AcpiMcfgInfo mcfg;
>  PcPciInfo pci;
>  uint8_t *u;
> +size_t aml_len = 0;
>  
>  acpi_get_cpu_info(&cpu);
>  acpi_get_pm_info(&pm);
> @@ -1474,13 +1489,20 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  dsdt = tables->table_data->len;
>  build_dsdt(tables->table_data, tables->linker, &misc);
>  
> +/* Count the size of the DSDT and SSDT, we will need it for legacy
> + * sizing of ACPI tables.
> + */
> +aml_len += tables->table_data->len - dsdt;
> +
>  /* ACPI tables pointed to by RSDT */
>  acpi_add_table(table_offsets, tables->table_data);
>  build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
>  
> +ssdt = tables->table_data->len;
>  acpi_add_table(table_offsets, tables->table_da

[Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest

2014-07-28 Thread Mikhail Ilin

Hi,

As it was posted earlier the output of reading /proc/self/maps is not
correct for a guest. There are some issues:

https://bugs.launchpad.net/qemu/+bug/1346784
http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03085.html
http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02793.html

The patch proposes: build /proc/self/maps doing a match against guest memory
translation table and output only that map records which are valid for guest
memory layout.

Patches in mentioned threads are not relevant and are covered by the current
patch.

We did some local tests for i386, x86_64 and arm targets. The approach
seems correct.


From 8479d3dd00194975d7016eeecba13ddf453e9647 Mon Sep 17 00:00:00 2001
From: Mikhail Ilyin 
Date: Mon, 28 Jul 2014 15:40:31 +0400
Subject: [PATCH] Build /proc/self/maps doing a match against guest memory
 translation table. Output only that map records which are valid for guest
 memory layout.

Signed-off-by: Mikhail Ilyin 
---
 include/exec/cpu-all.h |  2 ++
 linux-user/syscall.c   | 25 ++---
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..f9d132f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
 #define RESERVED_VA 0ul
 #endif

+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
+(1ul << 
TARGET_VIRT_ADDR_SPACE_BITS) - 1)

 #endif

 /* page related stuff */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..189a8c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)

 static int open_self_maps(void *cpu_env, int fd)
 {
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || 
defined(TARGET_UNICORE32)

 CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
 TaskState *ts = cpu->opaque;
-#endif
 FILE *fp;
 char *line = NULL;
 size_t len = 0;
@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
 if ((fields < 10) || (fields > 11)) {
 continue;
 }
-if (!strncmp(path, "[stack]", 7)) {
-continue;
-}
-if (h2g_valid(min) && h2g_valid(max)) {
+if (h2g_valid(min)) {
+int flags = page_get_flags(h2g(min));
+max = h2g_valid(max - 1) ? max : (uint64_t)g2h(GUEST_ADDR_MAX);
+if (page_check_range(h2g(min), max - min, flags) == -1) {
+continue;
+}
+if (h2g(min) == ts->info->stack_limit) {
+pstrcpy(path, sizeof(path), "  [stack]");
+}
 dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
 " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-h2g(min), h2g(max), flag_r, flag_w,
+h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
 flag_x, flag_p, offset, dev_maj, dev_min, inode,
 path[0] ? " " : "", path);
 }
@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
 free(line);
 fclose(fp);

-#if defined(TARGET_ARM) || defined(TARGET_M68K) || 
defined(TARGET_UNICORE32)

-dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0  [stack]\n",
-(unsigned long long)ts->info->stack_limit,
-(unsigned long long)(ts->info->start_stack +
- (TARGET_PAGE_SIZE - 1)) & 
TARGET_PAGE_MASK,

-(unsigned long long)0);
-#endif
-
 return 0;
 }

--
1.9.1



Re: [Qemu-devel] [PATCH v2 for-2.1 2/2] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 13:45, Michael S. Tsirkin ha scritto:
>> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>> + * -M pc-i440fx-2.0.
> 
> Let's just say 2.0 and earlier?

This would give the idea that 1.6 is broken, but it isn't.

>>  Even if the actual amount of AML generated grows
>> + * a little bit, there should be plenty of free space since the DSDT
>> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
>> + */
>> +#define ACPI_BUILD_CPU_AML_SIZE97
>> +#define ACPI_BUILD_BRIDGE_AML_SIZE 1875
> 
> Let's put _LEGACY_ somewhere here?

Ok.

>> +
>> +#define ACPI_BUILD_TABLE_SIZE  0x1
>> +
>>  typedef struct AcpiCpuInfo {
>>  DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>>  } AcpiCpuInfo;
>> @@ -87,6 +99,8 @@ typedef struct AcpiBuildPciBusHotplugState {
>>  struct AcpiBuildPciBusHotplugState *parent;
>>  } AcpiBuildPciBusHotplugState;
>>  
>> +unsigned bsel_alloc;
>> +
> 
> Patch will be better contained if instead of using a global
> bsel_alloc, we actually go and count the devices that
> have ACPI_PCIHP_PROP_BSEL.
> You can just scan all devices, or all pci devices, it
> should not matter.
> This way, this code will be local to the legacy path.

Ok.

> 
>>  static void acpi_get_dsdt(AcpiMiscInfo *info)
>>  {
>>  uint16_t *applesmc_sta;
>> @@ -759,8 +773,8 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
>>  static void acpi_set_pci_info(void)
>>  {
>>  PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
>> -unsigned bsel_alloc = 0;
>>  
>> +assert(bsel_alloc == 0);
>>  if (bus) {
>>  /* Scan all PCI buses. Set property to enable acpi based hotplug. */
>>  pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
>> @@ -1440,13 +1454,14 @@ static
>>  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>>  {
>>  GArray *table_offsets;
>> -unsigned facs, dsdt, rsdt;
>> +unsigned facs, ssdt, dsdt, rsdt;
>>  AcpiCpuInfo cpu;
>>  AcpiPmInfo pm;
>>  AcpiMiscInfo misc;
>>  AcpiMcfgInfo mcfg;
>>  PcPciInfo pci;
>>  uint8_t *u;
>> +size_t aml_len = 0;
>>  
>>  acpi_get_cpu_info(&cpu);
>>  acpi_get_pm_info(&pm);
>> @@ -1474,13 +1489,20 @@ void acpi_build(PcGuestInfo *guest_info, 
>> AcpiBuildTables *tables)
>>  dsdt = tables->table_data->len;
>>  build_dsdt(tables->table_data, tables->linker, &misc);
>>  
>> +/* Count the size of the DSDT and SSDT, we will need it for legacy
>> + * sizing of ACPI tables.
>> + */
>> +aml_len += tables->table_data->len - dsdt;
>> +
>>  /* ACPI tables pointed to by RSDT */
>>  acpi_add_table(table_offsets, tables->table_data);
>>  build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
>>  
>> +ssdt = tables->table_data->len;
>>  acpi_add_table(table_offsets, tables->table_data);
>>  build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
>> guest_info);
>> +aml_len += tables->table_data->len - ssdt;
>>  
>>  acpi_add_table(table_offsets, tables->table_data);
>>  build_madt(tables->table_data, tables->linker, &cpu, guest_info);
>> @@ -1513,12 +1535,53 @@ void acpi_build(PcGuestInfo *guest_info, 
>> AcpiBuildTables *tables)
>>  /* RSDP is in FSEG memory, so allocate it separately */
>>  build_rsdp(tables->rsdp, tables->linker, rsdt);
>>  
>> -/* We'll expose it all to Guest so align size to reduce
>> +/* We'll expose it all to Guest so we want to reduce
>>   * chance of size changes.
>>   * RSDP is small so it's easy to keep it immutable, no need to
>>   * bother with alignment.
>> + *
>> + * We used to align the tables to 4k, but of course this would
>> + * too simple to be enough.  4k turned out to be too small an
>> + * alignment very soon, and in fact it is almost impossible to
>> + * keep the table size stable for all (max_cpus, max_memory_slots)
>> + * combinations.  So the table size is always 64k for pc-i440fx-2.1
>> + * and we give an error if the table grows beyond that limit.
>> + *
>> + * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
>> + * that, we exploit the fact that QEMU 2.1 generates _smaller_ tables
>> + * than 2.0 and we can always pad the smaller tables with zeros.  We can
>> + * then use the exact size of the 2.0 tables.
>> + *
>> + * All this is for PIIX4, since QEMU 2.0 didn't support Q35 migration.
>>   */
>> -acpi_align_size(tables->table_data, 0x1000);
>> +if (guest_info->legacy_acpi_table_size) {
>> +/* Subtracting aml_len gives the size of fixed tables.  Then add the
>> + * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
>> + */
>> +int legacy_aml_len =
>> +guest_info->legacy_acpi_table_size +
>> +ACPI_BUILD_CPU_AML_SIZE * max_cpus +
>> +ACPI_BUILD_BRIDGE_AML_SIZE * (MAX(bsel_alloc, 1) - 1);
>> +int legacy_table_siz

Re: [Qemu-devel] [PATCH v12 1/4] spapr_pci: Make find_phb()/find_dev() public

2014-07-28 Thread Alexander Graf


On 16.07.14 02:20, Gavin Shan wrote:

From: Alexey Kardashevskiy 

This makes find_phb()/find_dev() public and changed its names
to spapr_pci_find_phb()/spapr_pci_find_dev() as they are going to
be used from other parts of QEMU such as VFIO DDW (dynamic DMA window)
or VFIO PCI error injection or VFIO EEH handling - in all these
cases there are RTAS calls which are addressed to BUID+config_addr
in IEEE1275 format.

Signed-off-by: Alexey Kardashevskiy 


Do we need this still?


Alex




Re: [Qemu-devel] [PATCH v12 3/4] headers: Update kernel header

2014-07-28 Thread Alexander Graf


On 16.07.14 03:40, Gavin Shan wrote:

On Wed, Jul 16, 2014 at 11:32:13AM +1000, Alexey Kardashevskiy wrote:

On 07/16/2014 11:16 AM, Gavin Shan wrote:

On Wed, Jul 16, 2014 at 11:09:44AM +1000, Alexey Kardashevskiy wrote:

On 07/16/2014 10:20 AM, Gavin Shan wrote:

This updates kernel header (vfio.h) for EEH support on VFIO PCI
devices.

Has this reached kernel upstream? The way linux headers update normally
happens is you have to run scripts/update-linux-headers.sh against some
linux kernel tag which you know that it won't change (like v3.16-rc5) and
post all the changes as a single patch. It is never a header update for a
specific feature, it is just an update.


The kernel part isn't merged yet. I guess that's for 3.17 merge window.
Ok, good to know scripts/update-linux-headers.sh. So this patch should
be dropped and some one run the script to update QEMU (linux-headers
directory) ?


Once your changes are in upstream kernel, you wait till kernel tree gets
new "v3.xx-rcX" tag, then you run the script and make a separate patch for
QEMU. Then you wait till it reaches QEMU upstream (because I do not know
who will pull it to what tree, look at git history) or ppc-next (if Alex
pulls it and you are basing your work on ppc-next) and then repost other
patches.


Thanks for detailed explaining, Alexey. I guess I have to suspend a bit
until "v3.17.rc1" is coming out.


It's also perfectly fine to keep the kernel header update inside your 
patch set, but please make sure to mention which commit it is against. 
If I pick it up, I will generate the headers myself though and drop your 
patch while applying the rest of the set.



Alex




Re: [Qemu-devel] [PATCH] pty: Fix byte loss bug when connecting to pty

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 13:39, Sebastian Tanase ha scritto:
> When trying to print data to the pty, we first check if it is connected.
> If not, we try to reconnect, but we drop the pending data even if we
> have successfully reconnected; this makes us lose the first byte of the very
> first transmission.
> This small fix addresses the issue by checking once more if the pty is 
> connected
> after having tried to reconnect.
> 
> Signed-off-by: Sebastian Tanase 
> ---
> 
> To reproduce the bug, launch a qemu image that has a parallel port (say lp0)
> and redirect it to a pty (-parallel pty). After the VM is launched,
> open the corresponding pty on your host (cat /dev/pts/X) and send some
> data from the VM to the host: echo "abcd" > /dev/lp0
> The first time, the received string will be "bcd" instead of "abcd".
> This bug can have important consequences if you try, for example,
> to send a postscript file from a printer within the VM. Losing the
> first character will render the .ps file unusable.
> ---
>  qemu-char.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 7acc03f..ce52d0f 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1160,7 +1160,9 @@ static int pty_chr_write(CharDriverState *chr, const 
> uint8_t *buf, int len)
>  if (!s->connected) {
>  /* guest sends data, check for (re-)connect */
>  pty_chr_update_read_handler_locked(chr);
> -return 0;
> +if (!s->connected) {
> +return 0;
> +}
>  }
>  return io_channel_send(s->fd, buf, len);
>  }
> 

Looks ok, though only for 2.2 and 2.1.1.  Gerd, can you take care of
this patch?

Paolo



Re: [Qemu-devel] [PULL for-2.1 0/3] Last minute patches

2014-07-28 Thread Juan Quintela
Peter Maydell  wrote:
> On 25 July 2014 15:22, Paolo Bonzini  wrote:
>> Since Igor hasn't sent his patches, and I'm leaving the office, I pushed
>> this to
>>
>>git://github.com/bonzini/qemu.git tags/for-upstream-full
>>
>> I don't know about tests/acpi-test-data/pc.  It makes sense that this
>> patch should modify something there, but:
>>
>> * "make check" passes
>>
>> * the test warns even before patch 1, for both the DSDT (modified by the
>> patch) and SSDT (which this series doesn't touch at all)
>>
>> * I cannot get it to pass, except by blindly copying the "actual" output
>> on the "expected" files
>>
>> * mst is on vacation and Marcel is off on Fridays
>>
>> Based on my understanding of the problem, it is not possible to fix the
>> bug without hacks like this one, and even reverting all patches in this
>> area would be more risky.
>
> Hmm. I'm not really sure what the right thing is, so what
> I'm planning to do is:
>  * just apply the qemu-char fix for now
>  * not tag -rc4 today
>  * see if things are clearer on Monday (I see Igor has now
>sent out a patchset)
>  * tag -rc4 Mon or Tues
>  * slip the release date a few days (not a big deal, I think)

I am reading both patch-sets.

I preffer very much Paolo solution to Igor one.

But I have to say that I don't understand PATCH 1 (neither before or
after the change).  Solution does what we should do, that is generate
the size that destination is expecting, and no simply blindy accept
packages that are smaller.

The compatibility bits of PATCH2 look ok (that ones, I can kind of
understand them).


Igor?

Later, Juan.



Re: [Qemu-devel] [Bug 1343827] [NEW] block.c: multiwrite_merge() truncates overlapping requests

2014-07-28 Thread Andrey Korolyov
On Fri, Jul 18, 2014 at 10:15 AM, Slava Pestov
 wrote:
> Public bug reported:
>
> If the list of requests passed to multiwrite_merge() contains two
> requests where the first is for a range of sectors that is a strict
> subset of the second's, the second request is truncated to end where the
> first starts, so the second half of the second request is lost.
>
> This is easy to reproduce by running fio against a virtio-blk device
> running on qemu 2.1.0-rc1 with the below fio script. At least with fio
> 2.0.13, the randwrite pass will issue overlapping bios to the block
> driver, which the kernel is happy to pass along to qemu:
>
> [global]
> randrepeat=0
> ioengine=libaio
> iodepth=64
> direct=1
> size=1M
> numjobs=1
> verify_fatal=1
> verify_dump=1
>
> filename=$dev
>
> [seqwrite]
> blocksize_range=4k-1M
> rw=write
> verify=crc32c-intel
>
> [randwrite]
> stonewall
> blocksize_range=4k-1M
> rw=randwrite
> verify=meta
>
> Here is a naive fix for the problem that simply avoids merging
> problematic requests. I guess a better solution would be to redo
> qemu_iovec_concat() to do the right thing.
>
> diff -ur old/qemu-2.1.0-rc2/block.c qemu-2.1.0-rc2/block.c
> --- old/qemu-2.1.0-rc2/block.c  2014-07-15 14:49:14.0 -0700
> +++ qemu-2.1.0-rc2/block.c  2014-07-17 23:03:14.224169741 -0700
> @@ -4460,7 +4460,9 @@
>  int64_t oldreq_last = reqs[outidx].sector + reqs[outidx].nb_sectors;
>
>  // Handle exactly sequential writes and overlapping writes.
> -if (reqs[i].sector <= oldreq_last) {
> +// If this request ends before the previous one, don't merge.
> +if (reqs[i].sector <= oldreq_last &&
> +reqs[i].sector + reqs[i].nb_sectors >= oldreq_last) {
>  merge = 1;
>  }
>
> ** 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/1343827
>
> Title:
>   block.c: multiwrite_merge() truncates overlapping requests
>
> Status in QEMU:
>   New
>
> Bug description:
>   If the list of requests passed to multiwrite_merge() contains two
>   requests where the first is for a range of sectors that is a strict
>   subset of the second's, the second request is truncated to end where
>   the first starts, so the second half of the second request is lost.
>
>   This is easy to reproduce by running fio against a virtio-blk device
>   running on qemu 2.1.0-rc1 with the below fio script. At least with fio
>   2.0.13, the randwrite pass will issue overlapping bios to the block
>   driver, which the kernel is happy to pass along to qemu:
>
>   [global]
>   randrepeat=0
>   ioengine=libaio
>   iodepth=64
>   direct=1
>   size=1M
>   numjobs=1
>   verify_fatal=1
>   verify_dump=1
>
>   filename=$dev
>
>   [seqwrite]
>   blocksize_range=4k-1M
>   rw=write
>   verify=crc32c-intel
>
>   [randwrite]
>   stonewall
>   blocksize_range=4k-1M
>   rw=randwrite
>   verify=meta
>
>   Here is a naive fix for the problem that simply avoids merging
>   problematic requests. I guess a better solution would be to redo
>   qemu_iovec_concat() to do the right thing.
>
>   diff -ur old/qemu-2.1.0-rc2/block.c qemu-2.1.0-rc2/block.c
>   --- old/qemu-2.1.0-rc2/block.c  2014-07-15 14:49:14.0 -0700
>   +++ qemu-2.1.0-rc2/block.c  2014-07-17 23:03:14.224169741 -0700
>   @@ -4460,7 +4460,9 @@
>int64_t oldreq_last = reqs[outidx].sector + 
> reqs[outidx].nb_sectors;
>
>// Handle exactly sequential writes and overlapping writes.
>   -if (reqs[i].sector <= oldreq_last) {
>   +// If this request ends before the previous one, don't merge.
>   +if (reqs[i].sector <= oldreq_last &&
>   +reqs[i].sector + reqs[i].nb_sectors >= oldreq_last) {
>merge = 1;
>}
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1343827/+subscriptions
>

Hello,

bug is still here in the master and can be easily reproduced (and, of
course, looks like blocker since data corruption takes place). Does
anyone have an idea on when the fix (at least suggested one) will be
merged?



Re: [Qemu-devel] [PULL for-2.1 0/3] Last minute patches

2014-07-28 Thread Igor Mammedov
On Mon, 28 Jul 2014 14:59:26 +0200
Juan Quintela  wrote:

> Peter Maydell  wrote:
> > On 25 July 2014 15:22, Paolo Bonzini  wrote:
> >> Since Igor hasn't sent his patches, and I'm leaving the office, I pushed
> >> this to
> >>
> >>git://github.com/bonzini/qemu.git tags/for-upstream-full
> >>
> >> I don't know about tests/acpi-test-data/pc.  It makes sense that this
> >> patch should modify something there, but:
> >>
> >> * "make check" passes
> >>
> >> * the test warns even before patch 1, for both the DSDT (modified by the
> >> patch) and SSDT (which this series doesn't touch at all)
> >>
> >> * I cannot get it to pass, except by blindly copying the "actual" output
> >> on the "expected" files
> >>
> >> * mst is on vacation and Marcel is off on Fridays
> >>
> >> Based on my understanding of the problem, it is not possible to fix the
> >> bug without hacks like this one, and even reverting all patches in this
> >> area would be more risky.
> >
> > Hmm. I'm not really sure what the right thing is, so what
> > I'm planning to do is:
> >  * just apply the qemu-char fix for now
> >  * not tag -rc4 today
> >  * see if things are clearer on Monday (I see Igor has now
> >sent out a patchset)
> >  * tag -rc4 Mon or Tues
> >  * slip the release date a few days (not a big deal, I think)
> 
> I am reading both patch-sets.
> 
> I preffer very much Paolo solution to Igor one.
> 
> But I have to say that I don't understand PATCH 1 (neither before or
> after the change).  Solution does what we should do, that is generate
> the size that destination is expecting, and no simply blindy accept
> packages that are smaller.
> 
> The compatibility bits of PATCH2 look ok (that ones, I can kind of
> understand them).
>
> 
> Igor?
These patches work for 2.0->2.1 for -M pc-1.7 (Ubuntu case), however they 
doesn't
for 1.7->2.1 (RHEL7 case). 
Also chasing exact size is a bit problematic considering that different iasl
versions produce differently sized AML output.

As far as I understood from chat on #qemu,
Michael is going to pick-up this series + extendable RAMBlock series +
disabling PCI bridge hotplug patch and may be cook additional one to make 
working
backwards migration.

> 
> Later, Juan.




Re: [Qemu-devel] [PATCH 0/4 v8] ppc: Add debug stub support

2014-07-28 Thread Alexander Graf


On 14.07.14 11:15, Bharat Bhushan wrote:

This patchset add support for
  - software breakpoint
  - h/w breakpoint
  - h/w watchpoint
   
Please find description in individual patch.


Thanks, applied (with minor edits in comments) to ppc-next-2.2.


Alex




Re: [Qemu-devel] [PATCH 3/3] ppc/spapr: Fix MAX_CPUS to 255

2014-07-28 Thread Alexander Graf


On 27.06.14 08:47, Nikunj A Dadhania wrote:

MAX_CPUS 256 is inconsistent with qemu supporting upto 255 cpus. This
MAX_CPUS number was percolated back to "virsh capabilities" with wrong
max_cpus.

Signed-off-by: Nikunj A Dadhania 


Nice one :). Thanks, applied to ppc-next-2.2.


Alex




Re: [Qemu-devel] [PATCH/RFC 0/5] s390x/kvm: track the logical cpu state in QEMU and propagate it to kvm

2014-07-28 Thread Alexander Graf


On 10.07.14 15:27, David Hildenbrand wrote:

This is the qemu part of kernel series "Let user space control the
cpu states"

Christian Borntraeger (1):
   update linux headers with with cpustate changes

David Hildenbrand (4):
   s390x/kvm: introduce proper states for s390 cpus
   s390x/kvm: proper use of the cpu states OPERATING and STOPPED
   s390x/kvm: test whether a cpu is STOPPED when checking "has_work"
   s390x/kvm: propagate s390 cpu state to kvm

  hw/s390x/ipl.c|   2 +-
  hw/s390x/s390-virtio.c|  32 --
  linux-headers/linux/kvm.h |   7 ++-
  target-s390x/cpu.c| 106 +++---
  target-s390x/cpu.h|  33 +--
  target-s390x/helper.c |  11 ++---
  target-s390x/kvm.c|  49 ++---
  trace-events  |   6 +++
  8 files changed, 179 insertions(+), 67 deletions(-)


Looks good to me

--
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


@all thought it was the final internal review :)


It's a perfectly good thing to say "looks good to me" in public too. The 
only major difference is that usually you would say "reviewed-by" ;).



Alex





Re: [Qemu-devel] [PATCH/RFC 0/5] s390x/kvm: track the logical cpu state in QEMU and propagate it to kvm

2014-07-28 Thread Alexander Graf

On 28.07.2014, at 15:43, Alexander Graf  wrote:

> 
> On 10.07.14 15:27, David Hildenbrand wrote:
 This is the qemu part of kernel series "Let user space control the
 cpu states"
 
 Christian Borntraeger (1):
   update linux headers with with cpustate changes
 
 David Hildenbrand (4):
   s390x/kvm: introduce proper states for s390 cpus
   s390x/kvm: proper use of the cpu states OPERATING and STOPPED
   s390x/kvm: test whether a cpu is STOPPED when checking "has_work"
   s390x/kvm: propagate s390 cpu state to kvm
 
  hw/s390x/ipl.c|   2 +-
  hw/s390x/s390-virtio.c|  32 --
  linux-headers/linux/kvm.h |   7 ++-
  target-s390x/cpu.c| 106 
 +++---
  target-s390x/cpu.h|  33 +--
  target-s390x/helper.c |  11 ++---
  target-s390x/kvm.c|  49 ++---
  trace-events  |   6 +++
  8 files changed, 179 insertions(+), 67 deletions(-)
 
>>> Looks good to me
>>> 
>>> --
>>> 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
>>> 
>> @all thought it was the final internal review :)
> 
> It's a perfectly good thing to say "looks good to me" in public too. The only 
> major difference is that usually you would say "reviewed-by" ;).

Meh - only realized after I sent this that all those patches are From: you. 
Then of course it's not useful ;)


Alex




Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread Alexander Graf


On 10.07.14 15:10, Christian Borntraeger wrote:

From: David Hildenbrand 

If a cpu is stopped, it must never be allowed to run and no interrupt may wake 
it
up. A cpu also has to be unhalted if it is halted and has work to do - this
scenario wasn't hit in kvm case yet, as only "disabled wait" is processed within
QEMU.

Signed-off-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
Reviewed-by: Christian Borntraeger 
Signed-off-by: Christian Borntraeger 


This looks like it's something that generic infrastructure should take 
care of, no? How does this work for the other archs? They always get an 
interrupt on the transition between !has_work -> has_work. Why don't we 
get one for s390x?



Alex




Re: [Qemu-devel] [PATCH v12 1/4] spapr_pci: Make find_phb()/find_dev() public

2014-07-28 Thread Alexey Kardashevskiy
On 07/28/2014 10:49 PM, Alexander Graf wrote:
> 
> On 16.07.14 02:20, Gavin Shan wrote:
>> From: Alexey Kardashevskiy 
>>
>> This makes find_phb()/find_dev() public and changed its names
>> to spapr_pci_find_phb()/spapr_pci_find_dev() as they are going to
>> be used from other parts of QEMU such as VFIO DDW (dynamic DMA window)
>> or VFIO PCI error injection or VFIO EEH handling - in all these
>> cases there are RTAS calls which are addressed to BUID+config_addr
>> in IEEE1275 format.
>>
>> Signed-off-by: Alexey Kardashevskiy 
> 
> Do we need this still?


Ufff. I missed when this patchet stopped needing this patch :)
Looks to me that this patchset does not need it so I'll repost it for DDW
later.


-- 
Alexey



[Qemu-devel] [PATCH v3 0/5] ACPI fixes for QEMU 2.1

2014-07-28 Thread Paolo Bonzini
v2->v3:
fix tests/acpi-test-data/pc/DSDT [Peter]
track down "make check" failure, fix it [patch 4, me]
split patch 2 in two parts [mst]
do not make bsel_alloc global [mst]
include Igor's bridge patch [mst, as discussed on IRC]

Igor Mammedov (1):
  pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is
disabled

Paolo Bonzini (4):
  acpi-dsdt: procedurally generate _PRT
  pc: hack for migration compatibility from QEMU 2.0
  pc: future-proof migration-compatibility of ACPI tables
  bios-tables-test: fix ASL normalization false positive

 hw/i386/acpi-build.c|  119 ++-
 hw/i386/acpi-dsdt.dsl   |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++
 hw/i386/pc_piix.c   |   19 +
 hw/i386/pc_q35.c|5 +
 include/hw/i386/pc.h|1 +
 tests/acpi-test-data/pc/DSDT|  Bin 4499 -> 2807 bytes
 tests/bios-tables-test.c|6 +-
 8 files changed, 288 insertions(+), 1862 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT

2014-07-28 Thread Paolo Bonzini
This replaces the _PRT constant with a method that computes it.

The problem is that the DSDT+SSDT have grown from 2.0 to 2.1,
enough to cross the 8k barrier (we align the ACPI tables to 4k
before putting them in fw_cfg).  This causes problems with
migration and the pc-i440fx-2.0 machine type.

The solution to the problem is to hardcode 64k as the limit,
but this doesn't solve the bug with pc-i440fx-2.0.  The fix will be
for QEMU 2.1 to use exactly the same size as QEMU 2.0 for the
ACPI tables.  First, however, we must make the actual AML
equal or smaller; to do this, rewrite _PRT in a way that saves
over 1k of bytecode.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-dsdt.dsl   |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++
 tests/acpi-test-data/pc/DSDT|  Bin 4499 -> 2807 bytes
 3 files changed, 148 insertions(+), 1852 deletions(-)

diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 3cc0ea0..6ba0170 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -181,57 +181,45 @@ DefinitionBlock (
 
 Scope(\_SB) {
 Scope(PCI0) {
-Name(_PRT, Package() {
-/* PCI IRQ routing table, example from ACPI 2.0a specification,
-   section 6.2.8.1 */
-/* Note: we provide the same info as the PCI routing
-   table of the Bochs BIOS */
-
-#define prt_slot(nr, lnk0, lnk1, lnk2, lnk3) \
-Package() { nr##, 0, lnk0, 0 }, \
-Package() { nr##, 1, lnk1, 0 }, \
-Package() { nr##, 2, lnk2, 0 }, \
-Package() { nr##, 3, lnk3, 0 }
-
-#define prt_slot0(nr) prt_slot(nr, LNKD, LNKA, LNKB, LNKC)
-#define prt_slot1(nr) prt_slot(nr, LNKA, LNKB, LNKC, LNKD)
-#define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA)
-#define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB)
-
-prt_slot0(0x),
-/* Device 1 is power mgmt device, and can only use irq 9 */
-prt_slot(0x0001, LNKS, LNKB, LNKC, LNKD),
-prt_slot2(0x0002),
-prt_slot3(0x0003),
-prt_slot0(0x0004),
-prt_slot1(0x0005),
-prt_slot2(0x0006),
-prt_slot3(0x0007),
-prt_slot0(0x0008),
-prt_slot1(0x0009),
-prt_slot2(0x000a),
-prt_slot3(0x000b),
-prt_slot0(0x000c),
-prt_slot1(0x000d),
-prt_slot2(0x000e),
-prt_slot3(0x000f),
-prt_slot0(0x0010),
-prt_slot1(0x0011),
-prt_slot2(0x0012),
-prt_slot3(0x0013),
-prt_slot0(0x0014),
-prt_slot1(0x0015),
-prt_slot2(0x0016),
-prt_slot3(0x0017),
-prt_slot0(0x0018),
-prt_slot1(0x0019),
-prt_slot2(0x001a),
-prt_slot3(0x001b),
-prt_slot0(0x001c),
-prt_slot1(0x001d),
-prt_slot2(0x001e),
-prt_slot3(0x001f),
-})
+Method (_PRT, 0) {
+Store(Package(128) {}, Local0)
+Store(Zero, Local1)
+While(LLess(Local1, 128)) {
+// slot = pin >> 2
+Store(ShiftRight(Local1, 2), Local2)
+
+// lnk = (slot + pin) & 3
+Store(And(Add(Local1, Local2), 3), Local3)
+If (LEqual(Local3, 0)) {
+Store(Package(4) { Zero, Zero, LNKD, Zero }, Local4)
+}
+If (LEqual(Local3, 1)) {
+// device 1 is the power-management device, needs SCI
+If (LEqual(Local1, 4)) {
+Store(Package(4) { Zero, Zero, LNKS, Zero }, 
Local4)
+} Else {
+Store(Package(4) { Zero, Zero, LNKA, Zero }, 
Local4)
+}
+}
+If (LEqual(Local3, 2)) {
+Store(Package(4) { Zero, Zero, LNKB, Zero }, Local4)
+}
+If (LEqual(Local3, 3)) {
+Store(Package(4) { Zero, Zero, LNKC, Zero }, Local4)
+}
+
+// Complete the interrupt routing entry:
+//Package(4) { 0x[slot], [pin], [link], 0) }
+
+Store(Or(ShiftLeft(Local2, 16), 0x), Index(Local4, 0))
+Store(And(Local1, 3),Index(Local4, 1))
+Store(Local4,Index(Local0, 
Local1))
+
+Increment(Local1)
+}
+
+Return(Local0)
+}
 }
 
 Field(PCI0.ISA.P40C, ByteAcc, NoLock, Preserve) {
diff 

[Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Paolo Bonzini
Changing the ACPI table size causes migration to break, and the memory
hotplug work opened our eyes on how horribly we were breaking things in
2.0 already.

The ACPI table size is rounded to the next 4k, which one would think
gives some headroom.  In practice this is not the case, because the user
can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
8 to the MADT) and so some "-smp" values will break the 4k boundary and
fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.

This patch concerns itself with fixing migration from QEMU 2.0.  It
computes the payload size of QEMU 2.0 and always uses that one.
The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size
should always be enough; non-AML tables can change depending on the
configuration (especially MADT, SRAT, HPET) but they remain the same
between QEMU 2.0 and 2.1, so we only compute our padding based on the
sizes of the SSDT and DSDT.

Migration from QEMU 1.7 should work for guests that have a number of CPUs
other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
broken from QEMU 1.7 to QEMU 2.0 in the same way, though.

The amount of AML for a bridge varies a little bit between 1872 and 1875
due to optimized number encodings.  Use the smallest value, and let any
extra chew away at the slack left by the shrinking of the DSDT.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 84 +---
 hw/i386/pc_piix.c| 19 
 hw/i386/pc_q35.c |  5 
 include/hw/i386/pc.h |  1 +
 4 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..7d2251f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -25,7 +25,9 @@
 #include 
 #include "qemu-common.h"
 #include "qemu/bitmap.h"
+#include "qemu/osdep.h"
 #include "qemu/range.h"
+#include "qemu/error-report.h"
 #include "hw/pci/pci.h"
 #include "qom/cpu.h"
 #include "hw/i386/pc.h"
@@ -52,6 +54,15 @@
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 
+/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
+ * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
+ * a little bit, there should be plenty of free space since the DSDT
+ * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
+ */
+#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
+#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872
+#define ACPI_BUILD_ALIGN_SIZE 0x1000
+
 typedef struct AcpiCpuInfo {
 DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -737,6 +748,27 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
 ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
 }
 
+static void *pci_for_each_bus_incr_func(PCIBus *bus, void *opaque)
+{
+unsigned *bsel_alloc = opaque;
+
+if (bus->qbus.allow_hotplug) {
+(*bsel_alloc)++;
+}
+
+return bsel_alloc;
+}
+
+static unsigned acpi_count_hotpluggable_pci_buses(void)
+{
+PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
+unsigned bus_count = 0;
+
+pci_for_each_bus_depth_first(bus, pci_for_each_bus_incr_func,
+ NULL, &bus_count);
+return bus_count;
+}
+
 /* Assign BSEL property to all buses.  In the future, this can be changed
  * to only assign to buses that support hotplug.
  */
@@ -1440,13 +1472,14 @@ static
 void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 {
 GArray *table_offsets;
-unsigned facs, dsdt, rsdt;
+unsigned facs, ssdt, dsdt, rsdt;
 AcpiCpuInfo cpu;
 AcpiPmInfo pm;
 AcpiMiscInfo misc;
 AcpiMcfgInfo mcfg;
 PcPciInfo pci;
 uint8_t *u;
+size_t aml_len = 0;
 
 acpi_get_cpu_info(&cpu);
 acpi_get_pm_info(&pm);
@@ -1474,13 +1507,20 @@ void acpi_build(PcGuestInfo *guest_info, 
AcpiBuildTables *tables)
 dsdt = tables->table_data->len;
 build_dsdt(tables->table_data, tables->linker, &misc);
 
+/* Count the size of the DSDT and SSDT, we will need it for legacy
+ * sizing of ACPI tables.
+ */
+aml_len += tables->table_data->len - dsdt;
+
 /* ACPI tables pointed to by RSDT */
 acpi_add_table(table_offsets, tables->table_data);
 build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
 
+ssdt = tables->table_data->len;
 acpi_add_table(table_offsets, tables->table_data);
 build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
guest_info);
+aml_len += tables->table_data->len - ssdt;
 
 acpi_add_table(table_offsets, tables->table_data);
 build_madt(tables->table_data, tables->linker, &cpu, guest_info);
@@ -1513,14 +1553,50 @@ void acpi_build(PcGuestInfo *guest_info, 
AcpiBuildTables *tables)
 /* RSDP is in FSEG memory, so allocate it separately */
 build_rsdp(tables->rsdp, tables->linker, rsdt);
 
-/* We'll expose it all to Guest so align size to redu

[Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables

2014-07-28 Thread Paolo Bonzini
This patch avoids that similar changes break QEMU again in the future.
QEMU will now hard-code 64k as the maximum ACPI table size, which
(despite being an order of magnitude smaller than 640k) should be enough
for everyone.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7d2251f..8d42eaf 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -63,6 +63,8 @@
 #define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1875
 #define ACPI_BUILD_ALIGN_SIZE 0x1000
 
+#define ACPI_BUILD_TABLE_SIZE 0x1
+
 typedef struct AcpiCpuInfo {
 DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1593,7 +1595,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 }
 g_array_set_size(tables->table_data, legacy_table_size);
 } else {
-acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
+if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
+/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. 
 */
+error_report("ACPI tables are larger than 64k.  Please remove");
+error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
+exit(1);
+}
+g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
 }
 
 acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
-- 
1.8.3.1





[Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive

2014-07-28 Thread Paolo Bonzini
My version of IASL (from RHEL7) puts two newlines between the head comment
and the DefinitionBlock property.  One was already removed because the
test uses sizeof instead of strlen, but the extra one breaks the detection
of DefinitionBlock.  Killing all newlines after the comment drops the
warning.

Signed-off-by: Paolo Bonzini 
---
 tests/bios-tables-test.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 62771f7..045eb27 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -487,7 +487,11 @@ static GString *normalize_asl(gchar *asl_code)
 /* strip comments (different generation days) */
 comment = g_strstr_len(asl->str, asl->len, COMMENT_END);
 if (comment) {
-asl = g_string_erase(asl, 0, comment + sizeof(COMMENT_END) - asl->str);
+comment += strlen(COMMENT_END);
+while (*comment == '\n') {
+comment++;
+}
+asl = g_string_erase(asl, 0, comment - asl->str);
 }
 
 /* strip def block name (it has file path in it) */
-- 
1.8.3.1





[Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled

2014-07-28 Thread Paolo Bonzini
From: Igor Mammedov 

Fixes migration regression from QEMU-1.7 to a newer QEMUs.
SSDT table size in QEMU-1.7 doesn't change regardless of
a number of PCI bridge devices present at startup.

However in QEMU-2.0 since addition of hotplug on PCI bridges,
each PCI bridge adds ~1875 bytes to SSDT table, including
pc-i440fx-1.7 machine type where PCI bridge hotplug disabled
via compat property.
It breaks migration from "QEMU-1.7" to "QEMU-2.[01] -M pc-i440fx-1.7"
since RAMBlock size of ACPI tables on target becomes larger
then on source and migration fails with:

"Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000"

error.

Fix this by generating AML only for PCI0 bus if
hotplug on PCI bridges is disabled and preserves PCI brigde
description in AML as it was done in QEMU-1.7 for pc-i440fx-1.7.

It will help to maintain size of SSDT static regardless of
number of PCI bridges on startup for pc-i440fx-1.7 machine type.

Signed-off-by: Igor Mammedov 
[Affect bus_count too. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8d42eaf..90f1b90 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -77,6 +77,7 @@ typedef struct AcpiMcfgInfo {
 typedef struct AcpiPmInfo {
 bool s3_disabled;
 bool s4_disabled;
+bool pcihp_bridge_en;
 uint8_t s4_val;
 uint16_t sci_int;
 uint8_t acpi_enable_cmd;
@@ -98,6 +99,7 @@ typedef struct AcpiBuildPciBusHotplugState {
 GArray *device_table;
 GArray *notify_table;
 struct AcpiBuildPciBusHotplugState *parent;
+bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
 static void acpi_get_dsdt(AcpiMiscInfo *info)
@@ -201,6 +203,9 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
NULL);
 pm->gpe0_blk_len = object_property_get_int(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
NULL);
+pm->pcihp_bridge_en =
+object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
+ NULL);
 }
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -802,11 +807,13 @@ static void acpi_set_pci_info(void)
 }
 
 static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
- AcpiBuildPciBusHotplugState *parent)
+ AcpiBuildPciBusHotplugState *parent,
+ bool pcihp_bridge_en)
 {
 state->parent = parent;
 state->device_table = build_alloc_array();
 state->notify_table = build_alloc_array();
+state->pcihp_bridge_en = pcihp_bridge_en;
 }
 
 static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
@@ -820,7 +827,7 @@ static void *build_pci_bus_begin(PCIBus *bus, void 
*parent_state)
 AcpiBuildPciBusHotplugState *parent = parent_state;
 AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
 
-build_pci_bus_state_init(child, parent);
+build_pci_bus_state_init(child, parent, parent->pcihp_bridge_en);
 
 return child;
 }
@@ -841,6 +848,14 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 GArray *method;
 bool bus_hotplug_support = false;
 
+/*
+skip bridge subtree creation if bridge hotplug is disabled
+to make it compatible with 1.7 machine type
+*/
+if (!child->pcihp_bridge_en && bus->parent_dev) {
+return;
+}
+
 if (bus->parent_dev) {
 op = 0x82; /* DeviceOp */
 build_append_nameseg(bus_table, "S%.02X_",
@@ -887,7 +902,8 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 pc = PCI_DEVICE_GET_CLASS(pdev);
 dc = DEVICE_GET_CLASS(pdev);
 
-if (pc->class_id == PCI_CLASS_BRIDGE_ISA || pc->is_bridge) {
+if (pc->class_id == PCI_CLASS_BRIDGE_ISA ||
+(pc->is_bridge && child->pcihp_bridge_en)) {
 set_bit(slot, slot_device_system);
 }
 
@@ -899,7 +915,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 }
 }
 
-if (!dc->hotpluggable || pc->is_bridge) {
+if (!dc->hotpluggable || (pc->is_bridge && child->pcihp_bridge_en)) {
 clear_bit(slot, slot_hotplug_enable);
 }
 }
@@ -1164,7 +1180,7 @@ build_ssdt(GArray *table_data, GArray *linker,
 bus = PCI_HOST_BRIDGE(pci_host)->bus;
 }
 
-build_pci_bus_state_init(&hotplug_state, NULL);
+build_pci_bus_state_init(&hotplug_state, NULL, 
pm->pcihp_bridge_en);
 
 if (bus) {
 /* Scan all PCI buses. Generate tables to support hotplug. */
@@ -1578,7 +1594,8 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 /* Subtracting aml_len gives the size of fixed tables.  Then add the
  * size of the PIIX4 DSDT/SSDT in QEMU 2.0.
  */
-   

Re: [Qemu-devel] [PATCH] e1000: Delay LSC until mask is active

2014-07-28 Thread Alexander Graf


On 03.07.14 22:18, Michael S. Tsirkin wrote:

On Thu, Jul 03, 2014 at 08:00:04PM +0200, Alexander Graf wrote:

On 03.07.14 19:57, Peter Maydell wrote:

On 3 July 2014 18:39, Alexander Graf  wrote:

Mac OS X reads ICR on every interrupt. When the IRQ line is shared, this may
result in a race where LSC is not interpreted yet, but already gets cleared.

The guest already has a way of telling us that it can interpret LSC events
though and that's via the interrupt mask register (IMS).

So if we just leave the LSC interrupt bit pending, but invisible to the guest
as long as it's not ready to receive LSC interrupts, we basically defer the
interrupt to the earliest point in time when the guest would know how to
handle it.

This would break any guests dealing with this in a polling
mode (ie "permanently leave interrupts masked and read
ICR periodically to find out whether anything interesting
has happened"), right?

If those guests would wait for a link detect event that way, yes.

Considering all the hackery we already have about link negotiation (delay it
until a random amount of ms passed) I'd say the breakage this patch fixes is
a lot more likely than a polling guest that waits for a link based on
ICR.LSC :).


Alex

Well that hackery was justified by the claim that real hardware behaves
this way: it has a random delay since it needs a bit of time to bring
the link up.

What's the justification here? How come this driver works with
real hardware?


Real hardware never shares interrupts? ;)


Alex





Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread David Hildenbrand
> 
> On 10.07.14 15:10, Christian Borntraeger wrote:
> > From: David Hildenbrand 
> >
> > If a cpu is stopped, it must never be allowed to run and no interrupt may 
> > wake it
> > up. A cpu also has to be unhalted if it is halted and has work to do - this
> > scenario wasn't hit in kvm case yet, as only "disabled wait" is processed 
> > within
> > QEMU.
> >
> > Signed-off-by: David Hildenbrand 
> > Reviewed-by: Cornelia Huck 
> > Reviewed-by: Christian Borntraeger 
> > Signed-off-by: Christian Borntraeger 
> 
> This looks like it's something that generic infrastructure should take 
> care of, no? How does this work for the other archs? They always get an 
> interrupt on the transition between !has_work -> has_work. Why don't we 
> get one for s390x?
> 
> 
> Alex
> 
> 

Well, we have the special case on s390 as a CPU that is in the STOPPED or the
CHECK STOP state may never run - even if there is an interrupt. It's
basically like this CPU has been switched off.

Imagine that it is tried to inject an interrupt into a stopped vcpu. It
will kick the stopped vcpu and thus lead to a call to
"kvm_arch_process_async_events()". We have to deny that this vcpu will ever
run as long as it is stopped. It's like a way to "suppress" the
interrupt for such a transition you mentioned.

Later, another vcpu might decide to turn that vcpu back on (by e.g. sending a
SIGP START to that vcpu).

I am not sure if such a mechanism/scenario is applicable to any other arch. They
all seem to reset the cs->halted flag if they know they are able to run (e.g.
due to an interrupt) - they have no such thing as "stopped cpus", only
"halted/waiting cpus".

David




Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 16:16, David Hildenbrand ha scritto:
> Later, another vcpu might decide to turn that vcpu back on (by e.g. sending a
> SIGP START to that vcpu).
> 
> I am not sure if such a mechanism/scenario is applicable to any other arch. 
> They
> all seem to reset the cs->halted flag if they know they are able to run (e.g.
> due to an interrupt) - they have no such thing as "stopped cpus", only
> "halted/waiting cpus".

On x86, INIT_RECEIVED is pretty much a stopped CPU.  It can only run
(and receive interrupts) after getting a special startup interrupt ("SIPI").

Paolo



Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread Alexander Graf

On 28.07.2014, at 16:16, David Hildenbrand  wrote:

>> 
>> On 10.07.14 15:10, Christian Borntraeger wrote:
>>> From: David Hildenbrand 
>>> 
>>> If a cpu is stopped, it must never be allowed to run and no interrupt may 
>>> wake it
>>> up. A cpu also has to be unhalted if it is halted and has work to do - this
>>> scenario wasn't hit in kvm case yet, as only "disabled wait" is processed 
>>> within
>>> QEMU.
>>> 
>>> Signed-off-by: David Hildenbrand 
>>> Reviewed-by: Cornelia Huck 
>>> Reviewed-by: Christian Borntraeger 
>>> Signed-off-by: Christian Borntraeger 
>> 
>> This looks like it's something that generic infrastructure should take 
>> care of, no? How does this work for the other archs? They always get an 
>> interrupt on the transition between !has_work -> has_work. Why don't we 
>> get one for s390x?
>> 
>> 
>> Alex
>> 
>> 
> 
> Well, we have the special case on s390 as a CPU that is in the STOPPED or the
> CHECK STOP state may never run - even if there is an interrupt. It's
> basically like this CPU has been switched off.
> 
> Imagine that it is tried to inject an interrupt into a stopped vcpu. It
> will kick the stopped vcpu and thus lead to a call to
> "kvm_arch_process_async_events()". We have to deny that this vcpu will ever
> run as long as it is stopped. It's like a way to "suppress" the
> interrupt for such a transition you mentioned.

An interrupt kick usually just means we go back into the main loop. From there 
we check the interrupt bitmap which interrupt to handle. Check out the handling 
code here:

  
http://git.qemu.org/?p=qemu.git;a=blob;f=cpu-exec.c;h=38e5f02a307523d99134f4e2e6c51683bb10b45b;hb=HEAD#l580

If you just check for the stopped state in here, do_interrupt() will never get 
called and thus the CPU shouldn't ever get executed. Unless I'm heavily 
mistaken :).

> 
> Later, another vcpu might decide to turn that vcpu back on (by e.g. sending a
> SIGP START to that vcpu).

Yes, in that case that other CPU generates a signal (a different bit in 
interrupt_request) and the first CPU would see that it has to wake up and wake 
up.

> I am not sure if such a mechanism/scenario is applicable to any other arch. 
> They
> all seem to reset the cs->halted flag if they know they are able to run (e.g.
> due to an interrupt) - they have no such thing as "stopped cpus", only
> "halted/waiting cpus".

There's not really much difference between the two. The only difference from a 
software point of view is that a "stopped" CPU has its external interrupt bits 
masked off, no?


Alex




Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread David Hildenbrand
> 
> On 28.07.2014, at 16:16, David Hildenbrand  wrote:
> 
> >> 
> >> On 10.07.14 15:10, Christian Borntraeger wrote:
> >>> From: David Hildenbrand 
> >>> 
> >>> If a cpu is stopped, it must never be allowed to run and no interrupt may 
> >>> wake it
> >>> up. A cpu also has to be unhalted if it is halted and has work to do - 
> >>> this
> >>> scenario wasn't hit in kvm case yet, as only "disabled wait" is processed 
> >>> within
> >>> QEMU.
> >>> 
> >>> Signed-off-by: David Hildenbrand 
> >>> Reviewed-by: Cornelia Huck 
> >>> Reviewed-by: Christian Borntraeger 
> >>> Signed-off-by: Christian Borntraeger 
> >> 
> >> This looks like it's something that generic infrastructure should take 
> >> care of, no? How does this work for the other archs? They always get an 
> >> interrupt on the transition between !has_work -> has_work. Why don't we 
> >> get one for s390x?
> >> 
> >> 
> >> Alex
> >> 
> >> 
> > 
> > Well, we have the special case on s390 as a CPU that is in the STOPPED or 
> > the
> > CHECK STOP state may never run - even if there is an interrupt. It's
> > basically like this CPU has been switched off.
> > 
> > Imagine that it is tried to inject an interrupt into a stopped vcpu. It
> > will kick the stopped vcpu and thus lead to a call to
> > "kvm_arch_process_async_events()". We have to deny that this vcpu will ever
> > run as long as it is stopped. It's like a way to "suppress" the
> > interrupt for such a transition you mentioned.
> 
> An interrupt kick usually just means we go back into the main loop. From 
> there we check the interrupt bitmap which interrupt to handle. Check out the 
> handling code here:
> 
>   
> http://git.qemu.org/?p=qemu.git;a=blob;f=cpu-exec.c;h=38e5f02a307523d99134f4e2e6c51683bb10b45b;hb=HEAD#l580
> 
> If you just check for the stopped state in here, do_interrupt() will never 
> get called and thus the CPU shouldn't ever get executed. Unless I'm heavily 
> mistaken :).

So you would rather move the check out of has_work() into the main loop in
cpu-exec.c and directly into kvm_arch_process_async_events()?

This would on the other hand lead to an unhalt of the vcpu in cpu_exec() on any
CPU_INTERRUPT_HARD. A VCPU might thus be unhalted although it is not able to 
run. Is okay?

Looking at cpu.c:cpu_thread_is_idle(), we would maybe return false, although we
are idle (because we are idle when we are stopped)?

My qemu kvm knowledge is way better than the qemu emulation knowledge, so I
appreciate any insights :)

> 
> > 
> > Later, another vcpu might decide to turn that vcpu back on (by e.g. sending 
> > a
> > SIGP START to that vcpu).
> 
> Yes, in that case that other CPU generates a signal (a different bit in 
> interrupt_request) and the first CPU would see that it has to wake up and 
> wake up.
> 
> > I am not sure if such a mechanism/scenario is applicable to any other arch. 
> > They
> > all seem to reset the cs->halted flag if they know they are able to run 
> > (e.g.
> > due to an interrupt) - they have no such thing as "stopped cpus", only
> > "halted/waiting cpus".
> 
> There's not really much difference between the two. The only difference from 
> a software point of view is that a "stopped" CPU has its external interrupt 
> bits masked off, no?

Well the difference is, that a STOPPED vcpu can be woken up by non-interrupt
like things (SIGP START) AND a special interrupt (SIGP RESTART - which is like
a "SIPI"++ as it performs a psw exchange - "NMI"). So we basically have two
paths that can lead to a state change. All interrupt bits may be in any
combination (SIGP RESTART interrupts can't be masked out, nor can SIGP START be
denied).

The other thing may be that on s390, each vcpu (including itself) can put
another vcpu into the STOPPED state - I assume that this is different for x86 "
INIT_RECEIVED". For this reason we have to watch out for bad race conditions
(e.g. multiple vcpus working on another vcpu)...

David

> 
> 
> Alex
> 




Re: [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Michael S. Tsirkin
On Mon, Jul 28, 2014 at 04:02:12PM +0200, Paolo Bonzini wrote:
> Changing the ACPI table size causes migration to break, and the memory
> hotplug work opened our eyes on how horribly we were breaking things in
> 2.0 already.
> 
> The ACPI table size is rounded to the next 4k, which one would think
> gives some headroom.  In practice this is not the case, because the user
> can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
> 8 to the MADT) and so some "-smp" values will break the 4k boundary and
> fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.
> 
> This patch concerns itself with fixing migration from QEMU 2.0.  It
> computes the payload size of QEMU 2.0 and always uses that one.
> The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size
> should always be enough; non-AML tables can change depending on the
> configuration (especially MADT, SRAT, HPET) but they remain the same
> between QEMU 2.0 and 2.1, so we only compute our padding based on the
> sizes of the SSDT and DSDT.
> 
> Migration from QEMU 1.7 should work for guests that have a number of CPUs
> other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
> broken from QEMU 1.7 to QEMU 2.0 in the same way, though.
> 
> The amount of AML for a bridge varies a little bit between 1872 and 1875
> due to optimized number encodings.  Use the smallest value, and let any
> extra chew away at the slack left by the shrinking of the DSDT.
> 
> Reviewed-by: Laszlo Ersek 
> Tested-by: Igor Mammedov 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/i386/acpi-build.c | 84 
> +---
>  hw/i386/pc_piix.c| 19 
>  hw/i386/pc_q35.c |  5 
>  include/hw/i386/pc.h |  1 +
>  4 files changed, 105 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebc5f03..7d2251f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -25,7 +25,9 @@
>  #include 
>  #include "qemu-common.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/osdep.h"
>  #include "qemu/range.h"
> +#include "qemu/error-report.h"
>  #include "hw/pci/pci.h"
>  #include "qom/cpu.h"
>  #include "hw/i386/pc.h"
> @@ -52,6 +54,15 @@
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
>  
> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> + * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> + * a little bit, there should be plenty of free space since the DSDT
> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> + */
> +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
> +#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872


Hmm this is wrong if some slot is occupied by a
non hotpluggable device, is it not?

> +#define ACPI_BUILD_ALIGN_SIZE 0x1000
> +
>  typedef struct AcpiCpuInfo {
>  DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>  } AcpiCpuInfo;
> @@ -737,6 +748,27 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
>  ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
>  }
>  
> +static void *pci_for_each_bus_incr_func(PCIBus *bus, void *opaque)
> +{
> +unsigned *bsel_alloc = opaque;
> +
> +if (bus->qbus.allow_hotplug) {
> +(*bsel_alloc)++;
> +}

Hmm I don't know why we do allow_hotplug in the
place where you copied this from :(

> +
> +return bsel_alloc;
> +}
> +
> +static unsigned acpi_count_hotpluggable_pci_buses(void)
> +{
> +PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
> +unsigned bus_count = 0;
> +
> +pci_for_each_bus_depth_first(bus, pci_for_each_bus_incr_func,
> + NULL, &bus_count);
> +return bus_count;
> +}
> +
>  /* Assign BSEL property to all buses.  In the future, this can be changed
>   * to only assign to buses that support hotplug.
>   */
> @@ -1440,13 +1472,14 @@ static
>  void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>  {
>  GArray *table_offsets;
> -unsigned facs, dsdt, rsdt;
> +unsigned facs, ssdt, dsdt, rsdt;
>  AcpiCpuInfo cpu;
>  AcpiPmInfo pm;
>  AcpiMiscInfo misc;
>  AcpiMcfgInfo mcfg;
>  PcPciInfo pci;
>  uint8_t *u;
> +size_t aml_len = 0;
>  
>  acpi_get_cpu_info(&cpu);
>  acpi_get_pm_info(&pm);
> @@ -1474,13 +1507,20 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  dsdt = tables->table_data->len;
>  build_dsdt(tables->table_data, tables->linker, &misc);
>  
> +/* Count the size of the DSDT and SSDT, we will need it for legacy
> + * sizing of ACPI tables.
> + */
> +aml_len += tables->table_data->len - dsdt;
> +
>  /* ACPI tables pointed to by RSDT */
>  acpi_add_table(table_offsets, tables->table_data);
>  build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
>  
> +ssdt = tables->table_data->len;
>  acpi_add_table(table_offsets, tables->table_data);
>  build_ssdt(tables->table_data, tables->linker, &c

Re: [Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 17:05, Michael S. Tsirkin ha scritto:
>> > +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>> > + * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
>> > + * a little bit, there should be plenty of free space since the DSDT
>> > + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
>> > + */
>> > +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
>> > +#define ACPI_BUILD_LEGACY_BRIDGE_AML_SIZE 1872
> 
> Hmm this is wrong if some slot is occupied by a
> non hotpluggable device, is it not?

I honestly have no idea.  If even more complicated code is needed, I'd
rather get rid of all this bridge stuff altogether (as far as computing
the AML size goes), and declare 2.0->2.1 migration broken if the VM has
bridges.

Paolo



[Qemu-devel] [PATCH 1/5] scsi-bus: prepare scsi_req_new for introduction of parse_cdb

2014-07-28 Thread Paolo Bonzini
The per-SCSIDevice parse_cdb callback must not be called if the
request will go through special SCSIReqOps, so detect the special
cases early enough.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 51 ++-
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4341754..ca4e9f3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -561,13 +561,38 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
   uint8_t *buf, void *hba_private)
 {
 SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
+const SCSIReqOps *ops;
 SCSIRequest *req;
-SCSICommand cmd;
+SCSICommand cmd = { .len = 0 };
+int ret;
+
+if ((d->unit_attention.key == UNIT_ATTENTION ||
+ bus->unit_attention.key == UNIT_ATTENTION) &&
+(buf[0] != INQUIRY &&
+ buf[0] != REPORT_LUNS &&
+ buf[0] != GET_CONFIGURATION &&
+ buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
+
+ /*
+  * If we already have a pending unit attention condition,
+  * report this one before triggering another one.
+  */
+ !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
+ops = &reqops_unit_attention;
+} else if (lun != d->lun ||
+   buf[0] == REPORT_LUNS ||
+   (buf[0] == REQUEST_SENSE && d->sense_len)) {
+ops = &reqops_target_command;
+} else {
+ops = NULL;
+}
 
-if (scsi_req_parse(&cmd, d, buf) != 0) {
+ret = scsi_req_parse(&cmd, d, buf);
+if (ret != 0) {
 trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
 req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
 } else {
+assert(cmd.len != 0);
 trace_scsi_req_parsed(d->id, lun, tag, buf[0],
   cmd.mode, cmd.xfer);
 if (cmd.lba != -1) {
@@ -577,25 +602,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 
 if (cmd.xfer > INT32_MAX) {
 req = scsi_req_alloc(&reqops_invalid_field, d, tag, lun, 
hba_private);
-} else if ((d->unit_attention.key == UNIT_ATTENTION ||
-   bus->unit_attention.key == UNIT_ATTENTION) &&
-  (buf[0] != INQUIRY &&
-   buf[0] != REPORT_LUNS &&
-   buf[0] != GET_CONFIGURATION &&
-   buf[0] != GET_EVENT_STATUS_NOTIFICATION &&
-
-   /*
-* If we already have a pending unit attention condition,
-* report this one before triggering another one.
-*/
-   !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
-req = scsi_req_alloc(&reqops_unit_attention, d, tag, lun,
- hba_private);
-} else if (lun != d->lun ||
-   buf[0] == REPORT_LUNS ||
-   (buf[0] == REQUEST_SENSE && d->sense_len)) {
-req = scsi_req_alloc(&reqops_target_command, d, tag, lun,
- hba_private);
+} else if (ops) {
+req = scsi_req_alloc(ops, d, tag, lun, hba_private);
 } else {
 req = scsi_device_alloc_req(d, tag, lun, buf, hba_private);
 }
@@ -1186,6 +1194,7 @@ static int scsi_req_parse(SCSICommand *cmd, SCSIDevice 
*dev, uint8_t *buf)
 {
 int rc;
 
+cmd->lba = -1;
 switch (buf[0] >> 5) {
 case 0:
 cmd->len = 6;
-- 
1.9.3





[Qemu-devel] [PATCH for-2.2 v2 0/5] scsi: enable passthrough of vendor-specific commands

2014-07-28 Thread Paolo Bonzini
Right now scsi-generic is parsing the CDB, in order to compute
the expected number of bytes to be transferred.  This is necessary
if DMA is done by the HBA via scsi_req_data, but it prevents executing
vendor-specific commands via scsi-generic because we don't know how
to parse them.

If DMA is delegated to the SCSI layer via get_sg_list, we know in
advance how many bytes the guest will want to receive and we can pass
the information straight from the guest to SG_IO.  In this case, it is
unnecessary to parse the CDB to get the same information.  scsi-disk needs
it to detect underruns and overruns, but scsi-generic and scsi-block can
just ask the HBA about the transfer direction and size.

This series introduces a new parse_cdb callback in both the device and
the HBA.  The latter is called by scsi_bus_parse_cdb, which devices can
call for passthrough requests in their implementation of parse_cdb.

Paolo

v1->v2: use the "right" CDB size for non-vendor-specific commands,
as some drivers and/or firmware expect that and complain
if you pass a READ(10) command in a 16-byte CDB.  Interdiff
here.

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f999bfa..6f4462b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -57,12 +57,14 @@ int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, 
uint8_t *buf,
void *hba_private)
 {
 SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+int rc;
 
+assert(cmd->len == 0);
+rc = scsi_req_parse_cdb(dev, cmd, buf);
 if (bus->info->parse_cdb) {
-return bus->info->parse_cdb(dev, cmd, buf, hba_private);
-} else {
-return scsi_req_parse_cdb(dev, cmd, buf);
+rc = bus->info->parse_cdb(dev, cmd, buf, hba_private);
 }
+return rc;
 }
 
 static SCSIRequest *scsi_device_alloc_req(SCSIDevice *s, uint32_t tag, 
uint32_t lun,
@@ -575,7 +577,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 const SCSIReqOps *ops;
 SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
 SCSIRequest *req;
-SCSICommand cmd;
+SCSICommand cmd = { .len = 0 };
 int ret;
 
 if ((d->unit_attention.key == UNIT_ATTENTION ||
@@ -609,6 +611,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
 req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
 } else {
+assert(cmd.len != 0);
 trace_scsi_req_parsed(d->id, lun, tag, buf[0],
   cmd.mode, cmd.xfer);
 if (cmd.lba != -1) {
@@ -1210,6 +1213,7 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, 
uint8_t *buf)
 {
 int rc;
 
+cmd->lba = -1;
 switch (buf[0] >> 5) {
 case 0:
 cmd->len = 6;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index e8f4c0c..2dd9255 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -411,9 +411,10 @@ static int virtio_scsi_parse_cdb(SCSIDevice *dev, 
SCSICommand *cmd,
 {
 VirtIOSCSIReq *req = hba_private;
 
-cmd->lba = -1;
-cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE);
-memcpy(cmd->buf, buf, cmd->len);
+if (cmd->len == 0) {
+cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE);
+memcpy(cmd->buf, buf, cmd->len);
+}
 
 /* Extract the direction and mode directly from the request, for
  * host device passthrough.

Paolo Bonzini (5):
  scsi-bus: prepare scsi_req_new for introduction of parse_cdb
  scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo
  scsi-block: extract scsi_block_is_passthrough
  scsi-block, scsi-generic: implement parse_cdb
  virtio-scsi: implement parse_cdb

 hw/scsi/scsi-bus.c | 74 ++
 hw/scsi/scsi-disk.c| 52 +++
 hw/scsi/scsi-generic.c |  7 +
 hw/scsi/virtio-scsi.c  | 25 +
 include/hw/scsi/scsi.h |  7 +
 5 files changed, 130 insertions(+), 35 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH 3/5] scsi-block: extract scsi_block_is_passthrough

2014-07-28 Thread Paolo Bonzini
This will be used for both scsi_block_new_request and the scsi-block
implementation of parse_cdb.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-disk.c | 38 ++
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d47ecd6..81b7276 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2501,12 +2501,8 @@ static int scsi_block_initfn(SCSIDevice *dev)
 return scsi_initfn(&s->qdev);
 }
 
-static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
-   uint32_t lun, uint8_t *buf,
-   void *hba_private)
+static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-
 switch (buf[0]) {
 case READ_6:
 case READ_10:
@@ -2523,9 +2519,9 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
 case WRITE_VERIFY_12:
 case WRITE_VERIFY_16:
 /* If we are not using O_DIRECT, we might read stale data from the
-* host cache if writes were made using other commands than these
-* ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
-* O_DIRECT everything must go through SG_IO.
+ * host cache if writes were made using other commands than these
+ * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
+ * O_DIRECT everything must go through SG_IO.
  */
 if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) {
 break;
@@ -2542,13 +2538,31 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice 
*d, uint32_t tag,
  * just make scsi-block operate the same as scsi-generic for them.
  */
 if (s->qdev.type != TYPE_ROM) {
-return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
-  hba_private);
+return false;
 }
+break;
+
+default:
+break;
 }
 
-return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
-  hba_private);
+return true;
+}
+
+
+static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
+   uint32_t lun, uint8_t *buf,
+   void *hba_private)
+{
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+if (scsi_block_is_passthrough(s, buf)) {
+return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
+  hba_private);
+} else {
+return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
+  hba_private);
+}
 }
 #endif
 
-- 
1.9.3





[Qemu-devel] [PATCH 4/5] scsi-block, scsi-generic: implement parse_cdb

2014-07-28 Thread Paolo Bonzini
The callback lets the bus provide the direction and transfer count
for passthrough commands, enabling passthrough of vendor-specific
commands.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c |  3 +--
 hw/scsi/scsi-disk.c| 14 ++
 hw/scsi/scsi-generic.c |  7 +++
 include/hw/scsi/scsi.h |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index d97d18a..6f4462b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,6 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -1210,7 +1209,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 return lba;
 }
 
-static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
 int rc;
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 81b7276..d55521d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2564,6 +2564,19 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice 
*d, uint32_t tag,
   hba_private);
 }
 }
+
+static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
+  uint8_t *buf, void *hba_private)
+{
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+
+if (scsi_block_is_passthrough(s, buf)) {
+return scsi_bus_parse_cdb(&s->qdev, cmd, buf, hba_private);
+} else {
+return scsi_req_parse_cdb(&s->qdev, cmd, buf);
+}
+}
+
 #endif
 
 #define DEFINE_SCSI_DISK_PROPERTIES()\
@@ -2672,6 +2685,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 sc->init = scsi_block_initfn;
 sc->destroy  = scsi_destroy;
 sc->alloc_req= scsi_block_new_request;
+sc->parse_cdb= scsi_block_parse_cdb;
 dc->fw_name = "disk";
 dc->desc = "SCSI block device passthrough";
 dc->reset = scsi_disk_reset;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 3733d2c..0b2ff90 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -490,6 +490,12 @@ static Property scsi_generic_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static int scsi_generic_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+  uint8_t *buf, void *hba_private)
+{
+return scsi_bus_parse_cdb(dev, cmd, buf, hba_private);
+}
+
 static void scsi_generic_class_initfn(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -498,6 +504,7 @@ static void scsi_generic_class_initfn(ObjectClass *klass, 
void *data)
 sc->init = scsi_generic_initfn;
 sc->destroy  = scsi_destroy;
 sc->alloc_req= scsi_new_request;
+sc->parse_cdb= scsi_generic_parse_cdb;
 dc->fw_name = "disk";
 dc->desc = "pass through generic scsi device (/dev/sg*)";
 dc->reset = scsi_generic_reset;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 4a0b860..a7a28e6 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -250,6 +250,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
+int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.9.3





[Qemu-devel] [PATCH 2/5] scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo

2014-07-28 Thread Paolo Bonzini
These callbacks will let devices do their own request parsing, or
defer it to the bus.  If the bus does not provide an implementation,
in turn, fall back to the default parsing routine.

Swap the first two arguments to scsi_req_parse, and rename it to
scsi_req_parse_cdb, for consistency.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 26 +++---
 include/hw/scsi/scsi.h |  6 ++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ca4e9f3..d97d18a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -9,7 +9,7 @@
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
@@ -54,6 +54,20 @@ static void scsi_device_destroy(SCSIDevice *s)
 }
 }
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+   void *hba_private)
+{
+SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+int rc;
+
+assert(cmd->len == 0);
+rc = scsi_req_parse_cdb(dev, cmd, buf);
+if (bus->info->parse_cdb) {
+rc = bus->info->parse_cdb(dev, cmd, buf, hba_private);
+}
+return rc;
+}
+
 static SCSIRequest *scsi_device_alloc_req(SCSIDevice *s, uint32_t tag, 
uint32_t lun,
   uint8_t *buf, void *hba_private)
 {
@@ -562,6 +576,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 {
 SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
 const SCSIReqOps *ops;
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
 SCSIRequest *req;
 SCSICommand cmd = { .len = 0 };
 int ret;
@@ -587,7 +602,12 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun,
 ops = NULL;
 }
 
-ret = scsi_req_parse(&cmd, d, buf);
+if (ops != NULL || !sc->parse_cdb) {
+ret = scsi_req_parse_cdb(d, &cmd, buf);
+} else {
+ret = sc->parse_cdb(d, &cmd, buf, hba_private);
+}
+
 if (ret != 0) {
 trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
 req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private);
@@ -1190,7 +1210,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd)
 return lba;
 }
 
-static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
+static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
 {
 int rc;
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1adb549..4a0b860 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -76,6 +76,8 @@ typedef struct SCSIDeviceClass {
 DeviceClass parent_class;
 int (*init)(SCSIDevice *dev);
 void (*destroy)(SCSIDevice *s);
+int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ void *hba_private);
 SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
   uint8_t *buf, void *hba_private);
 void (*unit_attention_reported)(SCSIDevice *s);
@@ -131,6 +133,8 @@ struct SCSIReqOps {
 struct SCSIBusInfo {
 int tcq;
 int max_channel, max_target, max_lun;
+int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+ void *hba_private);
 void (*transfer_data)(SCSIRequest *req, uint32_t arg);
 void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
 void (*cancel)(SCSIRequest *req);
@@ -244,6 +248,8 @@ void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
 
+int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
+   void *hba_private);
 void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
-- 
1.9.3





[Qemu-devel] [PATCH 5/5] virtio-scsi: implement parse_cdb

2014-07-28 Thread Paolo Bonzini
Enable passthrough of vendor-specific commands.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/virtio-scsi.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 0eb069a..2dd9255 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -406,6 +406,30 @@ static void virtio_scsi_command_complete(SCSIRequest *r, 
uint32_t status,
 virtio_scsi_complete_cmd_req(req);
 }
 
+static int virtio_scsi_parse_cdb(SCSIDevice *dev, SCSICommand *cmd,
+ uint8_t *buf, void *hba_private)
+{
+VirtIOSCSIReq *req = hba_private;
+
+if (cmd->len == 0) {
+cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE);
+memcpy(cmd->buf, buf, cmd->len);
+}
+
+/* Extract the direction and mode directly from the request, for
+ * host device passthrough.
+ */
+cmd->xfer = req->qsgl.size;
+if (cmd->xfer == 0) {
+cmd->mode = SCSI_XFER_NONE;
+} else if (iov_size(req->elem.in_sg, req->elem.in_num) > req->resp_size) {
+cmd->mode = SCSI_XFER_FROM_DEV;
+} else {
+cmd->mode = SCSI_XFER_TO_DEV;
+}
+return 0;
+}
+
 static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r)
 {
 VirtIOSCSIReq *req = r->hba_private;
@@ -658,6 +682,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 .change = virtio_scsi_change,
 .hotplug = virtio_scsi_hotplug,
 .hot_unplug = virtio_scsi_hot_unplug,
+.parse_cdb = virtio_scsi_parse_cdb,
 .get_sg_list = virtio_scsi_get_sg_list,
 .save_request = virtio_scsi_save_request,
 .load_request = virtio_scsi_load_request,
-- 
1.9.3




Re: [Qemu-devel] [PATCH v5 2/2] vmdk: Optimize cluster allocation

2014-07-28 Thread Stefan Hajnoczi
On Tue, May 27, 2014 at 04:49:22PM +0800, Fam Zheng wrote:
> +if (!bs->backing_hd) {
> +memset(whole_grain, 0,  skip_start_sector << BDRV_SECTOR_BITS);
> +memset(whole_grain + (skip_end_sector << BDRV_SECTOR_BITS), 0,
> +   cluster_bytes - (skip_end_sector << BDRV_SECTOR_BITS));
> +}
> +
> +assert(skip_end_sector <= sector_num + extent->cluster_sectors);

Does this assertion make sense?  skip_end_sector is a small number of
sectors (relative to start of cluster), while sector_num +
extent->cluster_sectors is a large absolute sector offset.

> +/**
> + * get_cluster_offset
> + *
> + * Look up cluster offset in extent file by sector number, and store in
> + * @cluster_offset.
> + *
> + * For flat extent, the start offset as parsed from the description file is

s/extent/extents/

> + * returned.
> + *
> + * For sparse extent, look up in L1, L2 table. If allocate is true, return an

s/extent/extents/

> + * offset for a new cluster and update L2 cache. If there is a backing file,
> + * COW is done before returning; otherwise, zeroes are written to the 
> allocated
> + * cluster. Both COW and zero writting skips the sector range

s/writting/writing/


pgpcGqEk_CN0h.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH for 2.1 V3] qemu-img info: show nocow info

2014-07-28 Thread Stefan Hajnoczi
On Wed, Jul 09, 2014 at 10:43:13AM +0800, Chunyan Liu wrote:
> Add nocow info in 'qemu-img info' output to show whether the file
> currently has NOCOW flag set or not.
> 
> Signed-off-by: Chunyan Liu 
> ---
> Changes:
>   - update output info to "NOCOW flag: set"
> 
>  block/qapi.c | 25 +
>  qapi/block-core.json |  5 -
>  2 files changed, 29 insertions(+), 1 deletion(-)

This patch was sent on July 9th, after the 2.1 soft freeze when we stop
merging new features.  Soft freeze was 17th of June.

Please resend for QEMU 2.2 and update the qapi-schema.json version
comment.

Thanks,
Stefan


pgpnaVyvL6A24.pgp
Description: PGP signature


[Qemu-devel] [PATCH v4 0/5] ACPI fixes for QEMU 2.1

2014-07-28 Thread Paolo Bonzini
v3->v4:
drop all pretense of supporting bridges [me]

v2->v3:
fix tests/acpi-test-data/pc/DSDT [Peter]
track down "make check" failure, fix it [patch 4, me]
split patch 2 in two parts [mst]
do not make bsel_alloc global [mst]
include Igor's bridge patch [mst, as discussed on IRC]

Igor Mammedov (1):
  pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is
disabled

Paolo Bonzini (4):
  acpi-dsdt: procedurally generate _PRT
  pc: hack for migration compatibility from QEMU 2.0
  pc: future-proof migration-compatibility of ACPI tables
  bios-tables-test: fix ASL normalization false positive

 hw/i386/acpi-build.c|   94 +-
 hw/i386/acpi-dsdt.dsl   |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++
 hw/i386/pc_piix.c   |   19 +
 hw/i386/pc_q35.c|5 +
 include/hw/i386/pc.h|1 +
 tests/acpi-test-data/pc/DSDT|  Bin 4499 -> 2807 bytes
 tests/bios-tables-test.c|6 +-
 8 files changed, 263 insertions(+), 1862 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 4/5] bios-tables-test: fix ASL normalization false positive

2014-07-28 Thread Paolo Bonzini
My version of IASL (from RHEL7) puts two newlines between the head comment
and the DefinitionBlock property.  Kill all newlines after the comment,
so that normalize_asl works properly.

Signed-off-by: Paolo Bonzini 
---
 tests/bios-tables-test.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 62771f7..045eb27 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -487,7 +487,11 @@ static GString *normalize_asl(gchar *asl_code)
 /* strip comments (different generation days) */
 comment = g_strstr_len(asl->str, asl->len, COMMENT_END);
 if (comment) {
-asl = g_string_erase(asl, 0, comment + sizeof(COMMENT_END) - asl->str);
+comment += strlen(COMMENT_END);
+while (*comment == '\n') {
+comment++;
+}
+asl = g_string_erase(asl, 0, comment - asl->str);
 }
 
 /* strip def block name (it has file path in it) */
-- 
1.8.3.1





[Qemu-devel] [PATCH 1/5] acpi-dsdt: procedurally generate _PRT

2014-07-28 Thread Paolo Bonzini
This replaces the _PRT constant with a method that computes it.

The problem is that the DSDT+SSDT have grown from 2.0 to 2.1,
enough to cross the 8k barrier (we align the ACPI tables to 4k
before putting them in fw_cfg).  This causes problems with
migration and the pc-i440fx-2.0 machine type.

The solution to the problem is to hardcode 64k as the limit,
but this doesn't solve the bug with pc-i440fx-2.0.  The fix will be
for QEMU 2.1 to use exactly the same size as QEMU 2.0 for the
ACPI tables.  First, however, we must make the actual AML
equal or smaller; to do this, rewrite _PRT in a way that saves
over 1k of bytecode.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-dsdt.dsl   |   90 +-
 hw/i386/acpi-dsdt.hex.generated | 1910 +++
 tests/acpi-test-data/pc/DSDT|  Bin 4499 -> 2807 bytes
 3 files changed, 148 insertions(+), 1852 deletions(-)

diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 3cc0ea0..6ba0170 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -181,57 +181,45 @@ DefinitionBlock (
 
 Scope(\_SB) {
 Scope(PCI0) {
-Name(_PRT, Package() {
-/* PCI IRQ routing table, example from ACPI 2.0a specification,
-   section 6.2.8.1 */
-/* Note: we provide the same info as the PCI routing
-   table of the Bochs BIOS */
-
-#define prt_slot(nr, lnk0, lnk1, lnk2, lnk3) \
-Package() { nr##, 0, lnk0, 0 }, \
-Package() { nr##, 1, lnk1, 0 }, \
-Package() { nr##, 2, lnk2, 0 }, \
-Package() { nr##, 3, lnk3, 0 }
-
-#define prt_slot0(nr) prt_slot(nr, LNKD, LNKA, LNKB, LNKC)
-#define prt_slot1(nr) prt_slot(nr, LNKA, LNKB, LNKC, LNKD)
-#define prt_slot2(nr) prt_slot(nr, LNKB, LNKC, LNKD, LNKA)
-#define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB)
-
-prt_slot0(0x),
-/* Device 1 is power mgmt device, and can only use irq 9 */
-prt_slot(0x0001, LNKS, LNKB, LNKC, LNKD),
-prt_slot2(0x0002),
-prt_slot3(0x0003),
-prt_slot0(0x0004),
-prt_slot1(0x0005),
-prt_slot2(0x0006),
-prt_slot3(0x0007),
-prt_slot0(0x0008),
-prt_slot1(0x0009),
-prt_slot2(0x000a),
-prt_slot3(0x000b),
-prt_slot0(0x000c),
-prt_slot1(0x000d),
-prt_slot2(0x000e),
-prt_slot3(0x000f),
-prt_slot0(0x0010),
-prt_slot1(0x0011),
-prt_slot2(0x0012),
-prt_slot3(0x0013),
-prt_slot0(0x0014),
-prt_slot1(0x0015),
-prt_slot2(0x0016),
-prt_slot3(0x0017),
-prt_slot0(0x0018),
-prt_slot1(0x0019),
-prt_slot2(0x001a),
-prt_slot3(0x001b),
-prt_slot0(0x001c),
-prt_slot1(0x001d),
-prt_slot2(0x001e),
-prt_slot3(0x001f),
-})
+Method (_PRT, 0) {
+Store(Package(128) {}, Local0)
+Store(Zero, Local1)
+While(LLess(Local1, 128)) {
+// slot = pin >> 2
+Store(ShiftRight(Local1, 2), Local2)
+
+// lnk = (slot + pin) & 3
+Store(And(Add(Local1, Local2), 3), Local3)
+If (LEqual(Local3, 0)) {
+Store(Package(4) { Zero, Zero, LNKD, Zero }, Local4)
+}
+If (LEqual(Local3, 1)) {
+// device 1 is the power-management device, needs SCI
+If (LEqual(Local1, 4)) {
+Store(Package(4) { Zero, Zero, LNKS, Zero }, 
Local4)
+} Else {
+Store(Package(4) { Zero, Zero, LNKA, Zero }, 
Local4)
+}
+}
+If (LEqual(Local3, 2)) {
+Store(Package(4) { Zero, Zero, LNKB, Zero }, Local4)
+}
+If (LEqual(Local3, 3)) {
+Store(Package(4) { Zero, Zero, LNKC, Zero }, Local4)
+}
+
+// Complete the interrupt routing entry:
+//Package(4) { 0x[slot], [pin], [link], 0) }
+
+Store(Or(ShiftLeft(Local2, 16), 0x), Index(Local4, 0))
+Store(And(Local1, 3),Index(Local4, 1))
+Store(Local4,Index(Local0, 
Local1))
+
+Increment(Local1)
+}
+
+Return(Local0)
+}
 }
 
 Field(PCI0.ISA.P40C, ByteAcc, NoLock, Preserve) {
diff 

[Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables

2014-07-28 Thread Paolo Bonzini
This patch avoids that similar changes break QEMU again in the future.
QEMU will now hard-code 64k as the maximum ACPI table size, which
(despite being an order of magnitude smaller than 640k) should be enough
for everyone.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a3d5822..25cf297 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -62,6 +62,8 @@
 #define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
 #define ACPI_BUILD_ALIGN_SIZE 0x1000
 
+#define ACPI_BUILD_TABLE_SIZE 0x1
+
 typedef struct AcpiCpuInfo {
 DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1569,7 +1571,13 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 }
 g_array_set_size(tables->table_data, legacy_table_size);
 } else {
-acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
+if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
+/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. 
 */
+error_report("ACPI tables are larger than 64k.  Please remove");
+error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
+exit(1);
+}
+g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
 }
 
 acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
-- 
1.8.3.1





[Qemu-devel] [PATCH 5/5] pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled

2014-07-28 Thread Paolo Bonzini
From: Igor Mammedov 

Fixes migration regression from QEMU-1.7 to a newer QEMUs.
SSDT table size in QEMU-1.7 doesn't change regardless of
a number of PCI bridge devices present at startup.

However in QEMU-2.0 since addition of hotplug on PCI bridges,
each PCI bridge adds ~1875 bytes to SSDT table, including
pc-i440fx-1.7 machine type where PCI bridge hotplug disabled
via compat property.
It breaks migration from "QEMU-1.7" to "QEMU-2.[01] -M pc-i440fx-1.7"
since RAMBlock size of ACPI tables on target becomes larger
then on source and migration fails with:

"Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000"

error.

Fix this by generating AML only for PCI0 bus if
hotplug on PCI bridges is disabled and preserves PCI brigde
description in AML as it was done in QEMU-1.7 for pc-i440fx-1.7.

It will help to maintain size of SSDT static regardless of
number of PCI bridges on startup for pc-i440fx-1.7 machine type.

Signed-off-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 25cf297..b92c531 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -76,6 +76,7 @@ typedef struct AcpiMcfgInfo {
 typedef struct AcpiPmInfo {
 bool s3_disabled;
 bool s4_disabled;
+bool pcihp_bridge_en;
 uint8_t s4_val;
 uint16_t sci_int;
 uint8_t acpi_enable_cmd;
@@ -97,6 +98,7 @@ typedef struct AcpiBuildPciBusHotplugState {
 GArray *device_table;
 GArray *notify_table;
 struct AcpiBuildPciBusHotplugState *parent;
+bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
 static void acpi_get_dsdt(AcpiMiscInfo *info)
@@ -200,6 +202,9 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
NULL);
 pm->gpe0_blk_len = object_property_get_int(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
NULL);
+pm->pcihp_bridge_en =
+object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
+ NULL);
 }
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -780,11 +785,13 @@ static void acpi_set_pci_info(void)
 }
 
 static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
- AcpiBuildPciBusHotplugState *parent)
+ AcpiBuildPciBusHotplugState *parent,
+ bool pcihp_bridge_en)
 {
 state->parent = parent;
 state->device_table = build_alloc_array();
 state->notify_table = build_alloc_array();
+state->pcihp_bridge_en = pcihp_bridge_en;
 }
 
 static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
@@ -798,7 +805,7 @@ static void *build_pci_bus_begin(PCIBus *bus, void 
*parent_state)
 AcpiBuildPciBusHotplugState *parent = parent_state;
 AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
 
-build_pci_bus_state_init(child, parent);
+build_pci_bus_state_init(child, parent, parent->pcihp_bridge_en);
 
 return child;
 }
@@ -819,6 +826,14 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 GArray *method;
 bool bus_hotplug_support = false;
 
+/*
+skip bridge subtree creation if bridge hotplug is disabled
+to make it compatible with 1.7 machine type
+*/
+if (!child->pcihp_bridge_en && bus->parent_dev) {
+return;
+}
+
 if (bus->parent_dev) {
 op = 0x82; /* DeviceOp */
 build_append_nameseg(bus_table, "S%.02X_",
@@ -865,7 +880,8 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 pc = PCI_DEVICE_GET_CLASS(pdev);
 dc = DEVICE_GET_CLASS(pdev);
 
-if (pc->class_id == PCI_CLASS_BRIDGE_ISA || pc->is_bridge) {
+if (pc->class_id == PCI_CLASS_BRIDGE_ISA ||
+(pc->is_bridge && child->pcihp_bridge_en)) {
 set_bit(slot, slot_device_system);
 }
 
@@ -877,7 +893,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 }
 }
 
-if (!dc->hotpluggable || pc->is_bridge) {
+if (!dc->hotpluggable || (pc->is_bridge && child->pcihp_bridge_en)) {
 clear_bit(slot, slot_hotplug_enable);
 }
 }
@@ -1142,7 +1158,7 @@ build_ssdt(GArray *table_data, GArray *linker,
 bus = PCI_HOST_BRIDGE(pci_host)->bus;
 }
 
-build_pci_bus_state_init(&hotplug_state, NULL);
+build_pci_bus_state_init(&hotplug_state, NULL, 
pm->pcihp_bridge_en);
 
 if (bus) {
 /* Scan all PCI buses. Generate tables to support hotplug. */
-- 
1.8.3.1




[Qemu-devel] [PATCH 2/5] pc: hack for migration compatibility from QEMU 2.0

2014-07-28 Thread Paolo Bonzini
Changing the ACPI table size causes migration to break, and the memory
hotplug work opened our eyes on how horribly we were breaking things in
2.0 already.

The ACPI table size is rounded to the next 4k, which one would think
gives some headroom.  In practice this is not the case, because the user
can control the ACPI table size (each CPU adds 97 bytes to the SSDT and
8 to the MADT) and so some "-smp" values will break the 4k boundary and
fail to migrate.  Similarly, PCI bridges add ~1870 bytes to the SSDT.

This patch concerns itself with fixing migration from QEMU 2.0.  It
computes the payload size of QEMU 2.0 and always uses that one.
The previous patch shrunk the ACPI tables enough that the QEMU 2.0 size
should always be enough; non-AML tables can change depending on the
configuration (especially MADT, SRAT, HPET) but they remain the same
between QEMU 2.0 and 2.1, so we only compute our padding based on the
sizes of the SSDT and DSDT.

Migration from QEMU 1.7 should work for guests that have a number of CPUs
other than 12, 13, 14, 54, 55, 56, 97, 98, 139, 140.  It was already
broken from QEMU 1.7 to QEMU 2.0 in the same way, though.

Even with this patch, QEMU 1.7 and 2.0 have two different ideas of
"-M pc-i440fx-2.0" when there are PCI bridges.  Igor sent a patch to
adopt the QEMU 1.7 definition.  I think distributions should apply
it if they move directly from QEMU 1.7 to 2.1+ without ever packaging
version 2.0.

Reviewed-by: Laszlo Ersek 
Tested-by: Igor Mammedov 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/acpi-build.c | 60 
 hw/i386/pc_piix.c| 19 +
 hw/i386/pc_q35.c |  5 +
 include/hw/i386/pc.h |  1 +
 4 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..a3d5822 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -25,7 +25,9 @@
 #include 
 #include "qemu-common.h"
 #include "qemu/bitmap.h"
+#include "qemu/osdep.h"
 #include "qemu/range.h"
+#include "qemu/error-report.h"
 #include "hw/pci/pci.h"
 #include "qom/cpu.h"
 #include "hw/i386/pc.h"
@@ -52,6 +54,14 @@
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 
+/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
+ * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
+ * a little bit, there should be plenty of free space since the DSDT
+ * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
+ */
+#define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
+#define ACPI_BUILD_ALIGN_SIZE 0x1000
+
 typedef struct AcpiCpuInfo {
 DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
 } AcpiCpuInfo;
@@ -1440,13 +1450,14 @@ static
 void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 {
 GArray *table_offsets;
-unsigned facs, dsdt, rsdt;
+unsigned facs, ssdt, dsdt, rsdt;
 AcpiCpuInfo cpu;
 AcpiPmInfo pm;
 AcpiMiscInfo misc;
 AcpiMcfgInfo mcfg;
 PcPciInfo pci;
 uint8_t *u;
+size_t aml_len = 0;
 
 acpi_get_cpu_info(&cpu);
 acpi_get_pm_info(&pm);
@@ -1474,13 +1485,20 @@ void acpi_build(PcGuestInfo *guest_info, 
AcpiBuildTables *tables)
 dsdt = tables->table_data->len;
 build_dsdt(tables->table_data, tables->linker, &misc);
 
+/* Count the size of the DSDT and SSDT, we will need it for legacy
+ * sizing of ACPI tables.
+ */
+aml_len += tables->table_data->len - dsdt;
+
 /* ACPI tables pointed to by RSDT */
 acpi_add_table(table_offsets, tables->table_data);
 build_fadt(tables->table_data, tables->linker, &pm, facs, dsdt);
 
+ssdt = tables->table_data->len;
 acpi_add_table(table_offsets, tables->table_data);
 build_ssdt(tables->table_data, tables->linker, &cpu, &pm, &misc, &pci,
guest_info);
+aml_len += tables->table_data->len - ssdt;
 
 acpi_add_table(table_offsets, tables->table_data);
 build_madt(tables->table_data, tables->linker, &cpu, guest_info);
@@ -1513,14 +1531,45 @@ void acpi_build(PcGuestInfo *guest_info, 
AcpiBuildTables *tables)
 /* RSDP is in FSEG memory, so allocate it separately */
 build_rsdp(tables->rsdp, tables->linker, rsdt);
 
-/* We'll expose it all to Guest so align size to reduce
+/* We'll expose it all to Guest so we want to reduce
  * chance of size changes.
  * RSDP is small so it's easy to keep it immutable, no need to
  * bother with alignment.
+ *
+ * We used to align the tables to 4k, but of course this would
+ * too simple to be enough.  4k turned out to be too small an
+ * alignment very soon, and in fact it is almost impossible to
+ * keep the table size stable for all (max_cpus, max_memory_slots)
+ * combinations.  So the table size is always 64k for pc-i440fx-2.1
+ * and we give an error if the table grows beyond that limit.
+ *
+ * We still have the problem of migrating from "-M pc-i440fx-2.0".  For
+ * that, we exploit the fac

[Qemu-devel] [PATCH 1/3] loader: add support for resizeable blobs

2014-07-28 Thread Michael S. Tsirkin
Support resizeable blobs: we allocate more memory than currently
available in the blob, which can later be filled in.

Signed-off-by: Michael S. Tsirkin 
---
 include/hw/loader.h   | 14 +++--
 include/hw/nvram/fw_cfg.h |  2 +-
 hw/core/loader.c  | 15 +
 hw/nvram/fw_cfg.c | 79 ---
 4 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..ecce654 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -55,9 +55,19 @@ extern bool rom_file_has_mr;
 int rom_add_file(const char *file, const char *fw_dir,
  hwaddr addr, int32_t bootindex,
  bool option_rom);
-void *rom_add_blob(const char *name, const void *blob, size_t len,
+void *rom_add_blob_resizeable(const char *name, const void *blob,
+  size_t len, size_t max_len,
+  hwaddr addr, const char *fw_file_name,
+  FWCfgReadCallback fw_callback,
+  void *callback_opaque);
+static inline void *rom_add_blob(const char *name, const void *blob, size_t 
len,
hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque);
+   FWCfgReadCallback fw_callback, void *callback_opaque)
+{
+return rom_add_blob_resizeable(name, blob, len, len, addr,
+   fw_file_name, fw_callback, callback_opaque);
+}
+
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
 size_t romsize, hwaddr addr);
 int rom_load_all(void);
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 72b1549..da4f5c5 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -75,7 +75,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, 
void *data,
  size_t len);
 void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
   FWCfgReadCallback callback, void 
*callback_opaque,
-  void *data, size_t len);
+  void *data, size_t len, size_t max_len);
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
 hwaddr crl_addr, hwaddr data_addr);
 
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..ad6ec67 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -720,9 +720,11 @@ err:
 return -1;
 }
 
-void *rom_add_blob(const char *name, const void *blob, size_t len,
-   hwaddr addr, const char *fw_file_name,
-   FWCfgReadCallback fw_callback, void *callback_opaque)
+void *rom_add_blob_resizeable(const char *name, const void *blob,
+  size_t len, size_t max_len,
+  hwaddr addr, const char *fw_file_name,
+  FWCfgReadCallback fw_callback,
+  void *callback_opaque)
 {
 Rom *rom;
 void *data = NULL;
@@ -730,9 +732,10 @@ void *rom_add_blob(const char *name, const void *blob, 
size_t len,
 rom   = g_malloc0(sizeof(*rom));
 rom->name = g_strdup(name);
 rom->addr = addr;
-rom->romsize  = len;
-rom->datasize = len;
+rom->romsize  = max_len;
+rom->datasize = max_len;
 rom->data = g_malloc0(rom->datasize);
+assert(len <= rom->datasize);
 memcpy(rom->data, blob, len);
 rom_insert(rom);
 if (fw_file_name && fw_cfg) {
@@ -748,7 +751,7 @@ void *rom_add_blob(const char *name, const void *blob, 
size_t len,
 
 fw_cfg_add_file_callback(fw_cfg, fw_file_name,
  fw_callback, callback_opaque,
- data, rom->romsize);
+ data, rom->romsize, rom->datasize);
 }
 return data;
 }
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b71d251..65f233e 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -38,6 +38,8 @@
 #define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
 
 typedef struct FWCfgEntry {
+uint32_t max_len;
+uint32_t reset_len;
 uint32_t len;
 uint8_t *data;
 void *callback_opaque;
@@ -57,6 +59,9 @@ struct FWCfgState {
 uint16_t cur_entry;
 uint32_t cur_offset;
 Notifier machine_ready;
+/* Entry length: used for migration */
+#define FW_CFG_LEN_ENTRIES (2 * FW_CFG_MAX_ENTRY)
+uint32_t len[FW_CFG_LEN_ENTRIES];
 };
 
 #define JPG_FILE 0
@@ -336,6 +341,13 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 static void fw_cfg_reset(DeviceState *d)
 {
 FWCfgState *s = FW_CFG(d);
+int i, j;
+
+for (i = 0; i < ARRAY_SIZE(s->entries); ++i) {
+for (j = 0; j < ARRAY_SIZE(s->entries[0]); ++j) {
+s->entries[i][j].len = s->entries[i][j].reset_len;
+}
+}
 
 fw_cfg_select(

[Qemu-devel] [PATCH 2/3] migration: load smaller RAMBlock to a bigger one if permitted

2014-07-28 Thread Michael S. Tsirkin
From: Igor Mammedov 

Add API to mark memory region as extend-able on migration,
to allow migration code to load smaller RAMBlock into
a bigger one on destination QEMU instance.

This will allow to fix broken migration from QEMU 1.7/2.0 to
QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
versions by marking ACPI tables ROM blob as extend-able.
So that smaller tables from previous version could be always
migrated to a bigger rom blob on new version.

Credits-for-idea: Michael S. Tsirkin 
Signed-off-by: Igor Mammedov 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/exec/memory.h   | 11 +++
 include/exec/ram_addr.h |  3 +++
 arch_init.c | 22 +-
 exec.c  |  8 
 memory.c|  5 +
 5 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..f96ddbb 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -894,6 +894,17 @@ bool memory_region_present(MemoryRegion *container, hwaddr 
addr);
 bool memory_region_is_mapped(MemoryRegion *mr);
 
 /**
+ * memory_region_permit_extendable_migration: marks #MemoryRegion
+ * as extendable on migration, allowing the migration code to load
+ * source memory block of smaller size than destination memory block
+ * at migration time
+ *
+ * @mr: a #MemoryRegion whose #RAMBlock should be marked as
+ *  extendable on migration
+ */
+void memory_region_permit_extendable_migration(MemoryRegion *mr);
+
+/**
  * memory_region_find: translate an address/size relative to a
  * MemoryRegion into a #MemoryRegionSection.
  *
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 6593be1..7a6b782 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -34,6 +34,9 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
 void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
+#define RAM_EXTENDABLE_ON_MIGRATE (1U << 31)
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr);
+
 static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
  ram_addr_t length,
  unsigned client)
diff --git a/arch_init.c b/arch_init.c
index 8ddaf35..2c0c238 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1071,11 +1071,23 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 if (!strncmp(id, block->idstr, sizeof(id))) {
-if (block->length != length) {
-error_report("Length mismatch: %s: " RAM_ADDR_FMT
- " in != " RAM_ADDR_FMT, id, length,
- block->length);
-ret =  -EINVAL;
+if (block->flags & RAM_EXTENDABLE_ON_MIGRATE) {
+if (block->length < length) {
+error_report("Length too big: %s: " 
RAM_ADDR_FMT
+ " in > " RAM_ADDR_FMT, id, length,
+ block->length);
+ret =  -EINVAL;
+} else {
+memset(block->host, 0, block->length);
+}
+} else {
+if (block->length != length) {
+error_report("Length mismatch: %s: "
+ RAM_ADDR_FMT " in != "
+ RAM_ADDR_FMT,
+ id, length, block->length);
+ret =  -EINVAL;
+}
 }
 break;
 }
diff --git a/exec.c b/exec.c
index 765bd94..02536f8e 100644
--- a/exec.c
+++ b/exec.c
@@ -1214,6 +1214,14 @@ void qemu_ram_unset_idstr(ram_addr_t addr)
 }
 }
 
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr)
+{
+RAMBlock *block = find_ram_block(addr);
+
+assert(block != NULL);
+block->flags |= RAM_EXTENDABLE_ON_MIGRATE;
+}
+
 static int memory_try_enable_merging(void *addr, size_t len)
 {
 if (!qemu_opt_get_bool(qemu_get_machine_opts(), "mem-merge", true)) {
diff --git a/memory.c b/memory.c
index 64d7176..744c746 100644
--- a/memory.c
+++ b/memory.c
@@ -1791,6 +1791,11 @@ bool memory_region_is_mapped(MemoryRegion *mr)
 return mr->container ? true : false;
 }
 
+void memory_region_permit_extendable_migration(MemoryRegion *mr)
+{
+qemu_ram_set_extendable_on_migration(mr->ram_addr);
+}
+
 MemoryRegionSection memory_region_find(MemoryRegion *mr,
hwaddr addr, uint64_t size)
 {
-- 
MST




[Qemu-devel] [PATCH 3/3] loader: mark MR for resizeable blobs as extendable

2014-07-28 Thread Michael S. Tsirkin
This makes migration from older QEMU versions
more robust.

Signed-off-by: Michael S. Tsirkin 
---
 hw/core/loader.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index ad6ec67..fc00a87 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -745,6 +745,13 @@ void *rom_add_blob_resizeable(const char *name, const void 
*blob,
 
 if (rom_file_has_mr) {
 data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+/* If there's padding at tail, blob is resizeable.
+ * Set flag to allow migration from older QEMU versions
+ * where this region could have been smaller.
+ */
+if (max_len != len) {
+memory_region_permit_extendable_migration(rom->mr);
+}
 } else {
 data = rom->data;
 }
-- 
MST




Re: [Qemu-devel] [PATCH v2] Tap: fix vcpu long time io blocking on tap

2014-07-28 Thread Stefan Hajnoczi
On Fri, Jul 18, 2014 at 09:33:42AM +, Wangkai (Kevin,C) wrote:
> fix vcpu long time io blocking on tap, when too many packets was delivered 
> to the guest os via tap interface.
> 
> --
> Signed-off-by: Wangkai 

Thanks, applied to my net-next tree:
https://github.com/stefanha/qemu/commits/net-next

The patch did not apply cleanly so I had to do it manually:

  Applying: Tap: fix vcpu long time io blocking on tap
  fatal: corrupt patch at line 32
  Repository lacks necessary blobs to fall back on 3-way merge.
  Cannot fall back to three-way merge.
  Patch failed at 0001 Tap: fix vcpu long time io blocking on tap

Please use git-send-email(1).

I also adjusted the commit message and doc comments to fit QEMU style
and for grammar.

Stefan


pgpliTTumalRp.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH/RFC 4/5] s390x/kvm: test whether a cpu is STOPPED when checking "has_work"

2014-07-28 Thread David Hildenbrand
> > 
> > On 28.07.2014, at 16:16, David Hildenbrand  wrote:
> > 
> > >> 
> > >> On 10.07.14 15:10, Christian Borntraeger wrote:
> > >>> From: David Hildenbrand 
> > >>> 
> > >>> If a cpu is stopped, it must never be allowed to run and no interrupt 
> > >>> may wake it
> > >>> up. A cpu also has to be unhalted if it is halted and has work to do - 
> > >>> this
> > >>> scenario wasn't hit in kvm case yet, as only "disabled wait" is 
> > >>> processed within
> > >>> QEMU.
> > >>> 
> > >>> Signed-off-by: David Hildenbrand 
> > >>> Reviewed-by: Cornelia Huck 
> > >>> Reviewed-by: Christian Borntraeger 
> > >>> Signed-off-by: Christian Borntraeger 
> > >> 
> > >> This looks like it's something that generic infrastructure should take 
> > >> care of, no? How does this work for the other archs? They always get an 
> > >> interrupt on the transition between !has_work -> has_work. Why don't we 
> > >> get one for s390x?
> > >> 
> > >> 
> > >> Alex
> > >> 
> > >> 
> > > 
> > > Well, we have the special case on s390 as a CPU that is in the STOPPED or 
> > > the
> > > CHECK STOP state may never run - even if there is an interrupt. It's
> > > basically like this CPU has been switched off.
> > > 
> > > Imagine that it is tried to inject an interrupt into a stopped vcpu. It
> > > will kick the stopped vcpu and thus lead to a call to
> > > "kvm_arch_process_async_events()". We have to deny that this vcpu will 
> > > ever
> > > run as long as it is stopped. It's like a way to "suppress" the
> > > interrupt for such a transition you mentioned.
> > 
> > An interrupt kick usually just means we go back into the main loop. From 
> > there we check the interrupt bitmap which interrupt to handle. Check out 
> > the handling code here:
> > 
> >   
> > http://git.qemu.org/?p=qemu.git;a=blob;f=cpu-exec.c;h=38e5f02a307523d99134f4e2e6c51683bb10b45b;hb=HEAD#l580
> > 
> > If you just check for the stopped state in here, do_interrupt() will never 
> > get called and thus the CPU shouldn't ever get executed. Unless I'm heavily 
> > mistaken :).
> 
> So you would rather move the check out of has_work() into the main loop in
> cpu-exec.c and directly into kvm_arch_process_async_events()?
> 
> This would on the other hand lead to an unhalt of the vcpu in cpu_exec() on 
> any
> CPU_INTERRUPT_HARD. A VCPU might thus be unhalted although it is not able to 
> run. Is okay?
> 
> Looking at cpu.c:cpu_thread_is_idle(), we would maybe return false, although 
> we
> are idle (because we are idle when we are stopped)?
> 
> My qemu kvm knowledge is way better than the qemu emulation knowledge, so I
> appreciate any insights :)
> 
> > 
> > > 
> > > Later, another vcpu might decide to turn that vcpu back on (by e.g. 
> > > sending a
> > > SIGP START to that vcpu).
> > 
> > Yes, in that case that other CPU generates a signal (a different bit in 
> > interrupt_request) and the first CPU would see that it has to wake up and 
> > wake up.
> > 
> > > I am not sure if such a mechanism/scenario is applicable to any other 
> > > arch. They
> > > all seem to reset the cs->halted flag if they know they are able to run 
> > > (e.g.
> > > due to an interrupt) - they have no such thing as "stopped cpus", only
> > > "halted/waiting cpus".
> > 
> > There's not really much difference between the two. The only difference 
> > from a software point of view is that a "stopped" CPU has its external 
> > interrupt bits masked off, no?
> 
> Well the difference is, that a STOPPED vcpu can be woken up by non-interrupt
> like things (SIGP START) AND a special interrupt (SIGP RESTART - which is like
> a "SIPI"++ as it performs a psw exchange - "NMI"). So we basically have two
> paths that can lead to a state change. All interrupt bits may be in any
> combination (SIGP RESTART interrupts can't be masked out, nor can SIGP START 
> be
> denied).
> 
> The other thing may be that on s390, each vcpu (including itself) can put
> another vcpu into the STOPPED state - I assume that this is different for x86 
> "
> INIT_RECEIVED". For this reason we have to watch out for bad race conditions
> (e.g. multiple vcpus working on another vcpu)...

Ah, sorry, just to clearify, a vcpu always sets itself to STOPPED, its the other
vcpus that trigger it (= interrupt-like).

David

> 
> David
> 
> > 
> > 
> > Alex
> > 
> 




[Qemu-devel] [PATCH 2/2] target-mips/translate.c: Add judgement for msb and lsb

2014-07-28 Thread Dongxue Zhang
Use 'if' to make sure the real msb greater than the lsb. As the compiler may
not do this.

Signed-off-by: Dongxue Zhang 
---
 target-mips/translate.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index c381366..e2cce31 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -3946,14 +3946,23 @@ static void gen_bitops (DisasContext *ctx, uint32_t 
opc, int rt,
 break;
 #if defined(TARGET_MIPS64)
 case OPC_DINSM:
+if (lsb > (msb + 32)) {
+goto fail;
+}
 gen_load_gpr(t0, rt);
 tcg_gen_deposit_tl(t0, t0, t1, lsb, msb + 32 - lsb + 1);
 break;
 case OPC_DINSU:
+if (lsb > msb) {
+goto fail;
+}
 gen_load_gpr(t0, rt);
 tcg_gen_deposit_tl(t0, t0, t1, lsb + 32, msb - lsb + 1);
 break;
 case OPC_DINS:
+if (lsb > msb) {
+goto fail;
+}
 gen_load_gpr(t0, rt);
 tcg_gen_deposit_tl(t0, t0, t1, lsb, msb - lsb + 1);
 break;
-- 
1.8.1.2




[Qemu-devel] [PATCH 1/2] target-mips/translate.c: Free TCG in OPC_DINSV

2014-07-28 Thread Dongxue Zhang
Free t0 and t1 in opcode OPC_DINSV.

Signed-off-by: Dongxue Zhang 
---
 target-mips/translate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index d7b8c4d..c381366 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -15300,6 +15300,9 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 gen_load_gpr(t1, rs);
 
 gen_helper_dinsv(cpu_gpr[rt], cpu_env, t1, t0);
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
 break;
 }
 default:/* Invalid */
-- 
1.8.1.2




Re: [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables

2014-07-28 Thread Michael S. Tsirkin
On Mon, Jul 28, 2014 at 05:34:16PM +0200, Paolo Bonzini wrote:
> This patch avoids that similar changes break QEMU again in the future.
> QEMU will now hard-code 64k as the maximum ACPI table size, which
> (despite being an order of magnitude smaller than 640k) should be enough
> for everyone.

Famous last words :) So what worries me here, is that we
are potentially breaking legal configurations for the
benefit of the minority that cares about cross-version
migration.

So I'm inclined to apply everything except this patch, and
instead, use the patches that I sent to make the
ram block very large, something like 1 Megabyte.

This localizes the pain to cross-version migration.


> 
> Reviewed-by: Laszlo Ersek 
> Tested-by: Igor Mammedov 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/i386/acpi-build.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a3d5822..25cf297 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -62,6 +62,8 @@
>  #define ACPI_BUILD_LEGACY_CPU_AML_SIZE97
>  #define ACPI_BUILD_ALIGN_SIZE 0x1000
>  
> +#define ACPI_BUILD_TABLE_SIZE 0x1
> +
>  typedef struct AcpiCpuInfo {
>  DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>  } AcpiCpuInfo;
> @@ -1569,7 +1571,13 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  }
>  g_array_set_size(tables->table_data, legacy_table_size);
>  } else {
> -acpi_align_size(tables->table_data, ACPI_BUILD_ALIGN_SIZE);
> +if (tables->table_data->len > ACPI_BUILD_TABLE_SIZE) {
> +/* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory 
> slots.  */
> +error_report("ACPI tables are larger than 64k.  Please remove");
> +error_report("CPUs, NUMA nodes, memory slots or PCI bridges.");
> +exit(1);
> +}
> +g_array_set_size(tables->table_data, ACPI_BUILD_TABLE_SIZE);
>  }
>  
>  acpi_align_size(tables->linker, ACPI_BUILD_ALIGN_SIZE);
> -- 
> 1.8.3.1
> 



Re: [Qemu-devel] nic is lost, in the virtual machine running

2014-07-28 Thread Stefan Hajnoczi
On Sat, Jul 19, 2014 at 03:31:00PM +0800, melolinux wrote:
> 1. /usr/bin/qemu-system-x86_64 -machine accel=kvm -name 
> a58970a2-10aa-4973-9261-7384b2f53221 -S -machine pc-0.14,accel=kvm,usb=off -m 
> 4096 -smp 4,sockets=1,cores=4,threads=1 -uuid 
> f3a2217f-ae5c-461b-a922-d37a3a98edc6 -no-user-config -nodefaults -chardev 
> socket,id=charmonitor,path=/var/lib/libvirt/qemu/a58970a2-10aa-4973-9261-7384b2f53221.monitor,server,nowait
>  -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime 
> -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device 
> virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive 
> file=/dev/493bdd93-6eb4-4499-a344-99cd0abef251/95eb250a-05ab-4da1-8673-8a616b4d0a6e,if=none,id=drive-ide0-0-0,format=qcow2,cache=writeback
>  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 
> -drive 
> file=/dev/493bdd93-6eb4-4499-a344-99cd0abef251/27d2df8a-dc00-4f6a-8669-f5dffc7a92d8,if=none,id=drive-ide0-0-1,format=qcow2,cache=writeback
>  -device ide-hd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -netdev 
> tap,ifname=tapa589700,script=no,id=hostnet0 -device 
> e1000,netdev=hostnet0,id=net0,mac=c6:b0:5c:de:54:39,bus=pci.0,addr=0x3 
> -chardev spicevmc,id=charchannel0,name=vdagent -device 
> virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
>  -device usb-tablet,id=input0 -spice 
> port=5900,addr=10.9.40.213,disable-ticketing,seamless-migration=on -vga qxl 
> -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=9437184 -device 
> intel-hda,id=sound0,bus=pci.0,addr=0x4 -device 
> hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device 
> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
> 
> 
> 2. Problem found :the client can't find the network device (It takes a long 
> time since we create the vm)
> 
> 3. we use "qemu-monitor-commond --hmp xx info pci" to check it and find that 
> we lost the ethernet controller.

There is not enough information in #2 and #3 to say what is going on.

What does lspci report inside the guest?

Does rmmod and modprobe of the NIC driver inside the guest bring the
interface back?

Did you check that QEMU's "info pci" reports the NIC when the guest has
booted but doesn't report it when the network becomes unavailable?  If
yes, then please check the libvirt log for hot unplug events
(/var/log/libvirt).

Stefan


pgpGud3TZbN1S.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 3/5] pc: future-proof migration-compatibility of ACPI tables

2014-07-28 Thread Paolo Bonzini
Il 28/07/2014 17:59, Michael S. Tsirkin ha scritto:
> On Mon, Jul 28, 2014 at 05:34:16PM +0200, Paolo Bonzini wrote:
>> This patch avoids that similar changes break QEMU again in the future.
>> QEMU will now hard-code 64k as the maximum ACPI table size, which
>> (despite being an order of magnitude smaller than 640k) should be enough
>> for everyone.
> 
> Famous last words :) So what worries me here, is that we
> are potentially breaking legal configurations for the
> benefit of the minority that cares about cross-version
> migration.
> 
> So I'm inclined to apply everything except this patch, and
> instead, use the patches that I sent to make the
> ram block very large, something like 1 Megabyte.

Even just 128k are enough for 160 VCPUs, 255 memory slots and 35-40 PCI
bridges.  And for 2.2 I'd rather move to the other model where all
user-defined elements (MADT, SSDT) are in a separate file and we
guarantee that *all* changes are versioned by machine type.

What do you think about just changing 64k->128k?  Your patch is a huge
amount of code for -rc4.

Paolo



Re: [Qemu-devel] [Bug 1347387] [NEW] while i was created the new virtual machine using qemu the following error was shown in fedora version 20

2014-07-28 Thread Stefan Hajnoczi
On Wed, Jul 23, 2014 at 04:43:27AM -, selvakumar wrote:
> Public bug reported:
> 
> [root@localhost pkgs]# qemu-img create virtualdisk.img 100M
> qemu-img: symbol lookup error: qemu-img: undefined symbol: glfs_discard_async

CCing Fedora qemu-kvm package maintainer.

Stefan

> [root@localhost pkgs]# qemu-i386 create virtualdisk.img 100M
> Error while loading create: No such file or directory
> 
> [root@localhost pkgs]# rpm -qa qemu-kvm libvirt
> qemu-kvm-1.6.2-6.fc20.x86_64
> libvirt-1.1.3.5-2.fc20.x86_64
> [root@localhost pkgs]# 
> 
> [root@localhost pkgs]# rpm -qa|grep *qemu*
> qemu-system-m68k-1.6.2-6.fc20.x86_64
> qemu-kvm-1.6.2-6.fc20.x86_64
> qemu-system-microblaze-1.6.2-6.fc20.x86_64
> ipxe-roms-qemu-20130517-3.gitc4bce43.fc20.noarch
> qemu-common-1.6.2-6.fc20.x86_64
> qemu-system-sh4-1.6.2-6.fc20.x86_64
> qemu-system-sparc-1.6.2-6.fc20.x86_64
> qemu-system-lm32-1.6.2-6.fc20.x86_64
> qemu-img-1.6.2-6.fc20.x86_64
> qemu-system-s390x-1.6.2-6.fc20.x86_64
> qemu-system-cris-1.6.2-6.fc20.x86_64
> qemu-1.6.2-6.fc20.x86_64
> qemu-system-xtensa-1.6.2-6.fc20.x86_64
> qemu-system-moxie-1.6.2-6.fc20.x86_64
> qemu-system-ppc-1.6.2-6.fc20.x86_64
> libvirt-daemon-driver-qemu-1.1.3.5-2.fc20.x86_64
> qemu-system-mips-1.6.2-6.fc20.x86_64
> qemu-system-alpha-1.6.2-6.fc20.x86_64
> qemu-guest-agent-1.6.1-2.fc20.x86_64
> qemu-user-1.6.2-6.fc20.x86_64
> qemu-system-x86-1.6.2-6.fc20.x86_64
> qemu-system-arm-1.6.2-6.fc20.x86_64
> qemu-system-unicore32-1.6.2-6.fc20.x86_64
> qemu-system-or32-1.6.2-6.fc20.x86_64
> [root@localhost pkgs]#
> 
> ** 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/1347387
> 
> Title:
>   while i was created the new virtual machine using qemu the following
>   error was shown in fedora version 20
> 
> Status in QEMU:
>   New
> 
> Bug description:
>   [root@localhost pkgs]# qemu-img create virtualdisk.img 100M
>   qemu-img: symbol lookup error: qemu-img: undefined symbol: 
> glfs_discard_async
>   [root@localhost pkgs]# qemu-i386 create virtualdisk.img 100M
>   Error while loading create: No such file or directory
> 
>   [root@localhost pkgs]# rpm -qa qemu-kvm libvirt
>   qemu-kvm-1.6.2-6.fc20.x86_64
>   libvirt-1.1.3.5-2.fc20.x86_64
>   [root@localhost pkgs]# 
> 
>   [root@localhost pkgs]# rpm -qa|grep *qemu*
>   qemu-system-m68k-1.6.2-6.fc20.x86_64
>   qemu-kvm-1.6.2-6.fc20.x86_64
>   qemu-system-microblaze-1.6.2-6.fc20.x86_64
>   ipxe-roms-qemu-20130517-3.gitc4bce43.fc20.noarch
>   qemu-common-1.6.2-6.fc20.x86_64
>   qemu-system-sh4-1.6.2-6.fc20.x86_64
>   qemu-system-sparc-1.6.2-6.fc20.x86_64
>   qemu-system-lm32-1.6.2-6.fc20.x86_64
>   qemu-img-1.6.2-6.fc20.x86_64
>   qemu-system-s390x-1.6.2-6.fc20.x86_64
>   qemu-system-cris-1.6.2-6.fc20.x86_64
>   qemu-1.6.2-6.fc20.x86_64
>   qemu-system-xtensa-1.6.2-6.fc20.x86_64
>   qemu-system-moxie-1.6.2-6.fc20.x86_64
>   qemu-system-ppc-1.6.2-6.fc20.x86_64
>   libvirt-daemon-driver-qemu-1.1.3.5-2.fc20.x86_64
>   qemu-system-mips-1.6.2-6.fc20.x86_64
>   qemu-system-alpha-1.6.2-6.fc20.x86_64
>   qemu-guest-agent-1.6.1-2.fc20.x86_64
>   qemu-user-1.6.2-6.fc20.x86_64
>   qemu-system-x86-1.6.2-6.fc20.x86_64
>   qemu-system-arm-1.6.2-6.fc20.x86_64
>   qemu-system-unicore32-1.6.2-6.fc20.x86_64
>   qemu-system-or32-1.6.2-6.fc20.x86_64
>   [root@localhost pkgs]#
> 
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1347387/+subscriptions
> 


pgpgvyCSnx7qc.pgp
Description: PGP signature


  1   2   >