Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

2015-04-09 Thread Liviu Ionescu

> On 09 Apr 2015, at 20:58, Peter Maydell  wrote:
> 
> On 9 April 2015 at 18:41, Stefan Weil  wrote:
>> I think -mthreads is essential (needed for thread local storage),
>> and -D_POSIX=1 is also needed for 64 bit builds.
> 
> My 64-bit builds worked without either of these things...

I tried with these settings and it still does not work.

> On 09 Apr 2015, at 20:41, Stefan Weil  wrote:
> 
> Maybe you can try my QEMU fork from http://repo.or.cz/w/qemu/ar7.git
> and see whether it works for you. My binaries are based on that code.
> If it works, we have to look which patches are missing in the official
> QEMU version.

there are probably hundreds of differences between your fork ant the official 
branch.

in 'configure' too there are quite a lot of differences. apart from the above 
CFLAGS, I could not identify something special.


regards,

Liviu




Re: [Qemu-devel] [PATCH 00/14] Add memory attributes and use them in ARM

2015-04-09 Thread Edgar E. Iglesias
On Tue, Apr 07, 2015 at 09:09:46PM +0100, Peter Maydell wrote:
> Following from my previous RFC about transaction memory attributes,
> here's some code I think is good enough to drop the 'RFC' tag :-)
> (read: I would like to land this when master reopens for 2.4.)
> 
> I've included both the changes to the core memory system code
> and the target-arm changes as a usage example, but the ARM stuff
> is all at the end of the series, so if we want to split it and
> take it via separate subtrees that's fine.

Hi Peter,

More of a general comment. This is maybe follow-up patches but
at somepoint we will need to add attributes to the IOMMU
translate functions.

Thanks,
Edgar


> 
> I think I have followed the outcome of our discussions on the
> RFC; please let me know if I got confused or missed something.
> What we have here is:
>  * MemoryRegions can provide read_with_attrs and write_with_attrs
>so they can get memory attributes and return a success/error
>indication
>  * the attributes and error indication are plumbed through the
>core memory system code
>  * new functions address_space_ld*/st* are provided which are
>like the old ld/st*_phys but have extra args for MemTxAttrs
>and MemTxResult*
>  * callers have been auto-converted from the old ld/st*_phys
>unless they were using the CPUState::as address space
>[those will be moved to some cpu-specific API later]
>  * TCG frontends can use tlb_set_page_with_attrs() to provide
>attributes when they add an entry to the TLB
>  * two attributes: MEMTXATTRS_SECURE [ARM TrustZone secure access]
>and MEMTXATTRS_USER [access is unprivileged], both implemented
>for the ARM CPU frontend (these both correspond to AMBA/AXI
>bus sideband signals, more or less)
> 
> 
> I believe this code contains enough changes that all the memory
> transactions issued by the ARM CPU will correctly be marked as
> S or NS. Obviously nothing currently pays attention to this, but
> the patches to make the GIC model support TrustZone can be easily
> wired up to this.
> 
> The diffstat's quite big but the biggest patch is a Coccinelle
> generated automated rename of the callers of ld/st*_phys to
> address_space_ld/st* where they don't use the CPUState::as.
> 
> thanks
> -- PMM
> 
> Peter Maydell (14):
>   memory: Define API for MemoryRegionOps to take attrs and return status
>   memory: Add MemTxAttrs, MemTxResult to io_mem_read and io_mem_write
>   Make CPU iotlb a structure rather than a plain hwaddr
>   Add MemTxAttrs to the IOTLB
>   exec.c: Convert subpage memory ops to _with_attrs
>   exec.c: Make address_space_rw take transaction attributes
>   exec.c: Add new address_space_ld*/st* functions
>   Switch non-CPU callers from ld/st*_phys to address_space_ld/st*
>   exec.c: Capture the memory attributes for a watchpoint hit
>   target-arm: Honour NS bits in page tables
>   target-arm: Use correct memory attributes for page table walks
>   target-arm: Add user-mode transaction attribute
>   target-arm: Use attribute info to handle user-only watchpoints
>   target-arm: Check watchpoints against CPU security state
> 
>  cputlb.c  |  22 +-
>  dma-helpers.c |   3 +-
>  exec.c| 418 
> +-
>  hw/alpha/dp264.c  |   9 +-
>  hw/alpha/typhoon.c|   3 +-
>  hw/arm/boot.c |   6 +-
>  hw/arm/highbank.c |  12 +-
>  hw/dma/pl080.c|  20 +-
>  hw/dma/sun4m_iommu.c  |   3 +-
>  hw/i386/intel_iommu.c |   3 +-
>  hw/mips/mips_jazz.c   |   6 +-
>  hw/pci-host/apb.c |   3 +-
>  hw/pci-host/prep.c|   6 +-
>  hw/pci/msi.c  |   3 +-
>  hw/pci/msix.c |   3 +-
>  hw/s390x/css.c|  19 +-
>  hw/s390x/s390-pci-bus.c   |   9 +-
>  hw/s390x/s390-pci-inst.c  |   7 +-
>  hw/s390x/s390-virtio-bus.c|  73 ---
>  hw/s390x/s390-virtio.c|   4 +-
>  hw/s390x/virtio-ccw.c |  87 +---
>  hw/sh4/r2d.c  |   6 +-
>  hw/timer/hpet.c   |   5 +-
>  hw/vfio/pci.c |   4 +-
>  include/exec/cpu-defs.h   |  15 +-
>  include/exec/exec-all.h   |   7 +-
>  include/exec/memattrs.h   |  40 
>  include/exec/memory.h | 128 +++-
>  include/qom/cpu.h |   2 +
>  include/sysemu/dma.h  |   3 +-
>  ioport.c  |  16 +-
>  kvm-all.c |   3 +-
>  memory.c  | 212 ---
>  monitor.c |   3 +-
>  scripts/coverity-model.c  |   8 +-
>  softmmu_template.h|  36 ++--
>  target-arm/helper.c   | 134 ++--
>  target-arm/op_helper.c|  29 +--
>  target-i386/arch_memory_map

Re: [Qemu-devel] [PATCH 10/14] target-arm: Honour NS bits in page tables

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 12:23, Edgar E. Iglesias  wrote:
> On Tue, Apr 07, 2015 at 09:09:56PM +0100, Peter Maydell wrote:
>>
>> +if (regime_is_secure(env, mmu_idx)) {
>> +/* The page table entries may downgrade this to non-secure, but
>> + * cannot upgrade an non-secure translation regime's attributes
>> + * to secure.
>> + */
>> +*attrs = MEMTXATTRS_SECURE;
>> +} else {
>> +*attrs = 0;
>> +}
>
>
> Should this initialization maybe be done from some kind of secure and NS
> per CPU attribute template?
> What I'm trying to get to is that the machine description may want to
> control some attributes like for example the master-id.
>
> Or did you have another mechanism in mind for that?

I wasn't sure whether we should be adding the tx master ID
in target-specific code or if there was a place to OR it in
in generic code. So I left it out for the moment, since I
don't have an immediate use case for it, and I wanted to
keep the series from ballooning in size too much. If we
end up deciding to do it here then it would be a pretty
simple change to make later.

> In the hack I'm using, CPU code doesn't actually edit the attributes.
> There are a set of attribute objects that board code sets up and the CPU
> selects among them depending on it's state. Once the attributes hit
> tlb_set_page_with_attrs a copy is made into the IOTLB so that
> IOMMUs can modify the actual value in the IOTLB as they translate().
>
> This makes it easy for board code to for example make a non TZ
> capable CPU look as either secure or non-secure.

I think we should probably do this sort of thing using CPU
QOM properties that the board can set appropriately on CPU
creation to influence CPU behaviour (eg a property to set
a tx master ID value, with default being the cpu number).

thanks
-- PMM



[Qemu-devel] [PATCH 2/2] qdev: Free property names after registering gpio aliases

2015-04-09 Thread Eduardo Habkost
Now that object_property_add_alias() strdup()s target_name, we can free
the property names in qdev_pass_gpios().

Signed-off-by: Eduardo Habkost 
---
 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..6dca6cb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -563,6 +563,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState 
*container,
 object_property_add_alias(OBJECT(container), propname,
   OBJECT(dev), propname,
   &error_abort);
+g_free(propname);
 }
 for (i = 0; i < ngl->num_out; i++) {
 const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
@@ -571,6 +572,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState 
*container,
 object_property_add_alias(OBJECT(container), propname,
   OBJECT(dev), propname,
   &error_abort);
+g_free(propname);
 }
 QLIST_REMOVE(ngl, node);
 QLIST_INSERT_HEAD(&container->gpios, ngl, node);
-- 
2.1.0




Re: [Qemu-devel] [Qemu-block] [RFC] Intermediate block mirroring

2015-04-09 Thread Alberto Garcia
On Thu, Apr 09, 2015 at 11:39:28AM +0100, Stefan Hajnoczi wrote:

> > The goal would be to convert this:
> > 
> >[A] -> [B] -> [C] -> [D]
> > 
> > into this:
> > 
> >[A] -> [B] -> [X] -> [D]
> > 
> > where [D] is the active image and [X] would be a copy of [C]. The
> > latter would be unlinked from the chain.
> > 
> > A use case would be to move disk images across different storage
> > backends.
> 
> The simple solution to that problem is:
> 
> Assumption: backing files are read only.  (True in most cases.)
> 
> 1. Copy the backing files using cp(1) or another method.
> 2. Issue QMP 'change-backing-file' command so that [D] uses [X] instead
>of [C].
> 
> So it can be done today already.

So do you think it would be better to implement this somewhere else?
The code that I have for QEMU is quite simple, the actual algorithm
doesn't change, it only needs to do the changing of backing files in
mirror_exit().

Berto



Re: [Qemu-devel] Getting VM state from outside QEMU?

2015-04-09 Thread Paulo Ricardo Paz Vital
Sorry guys only checked the emails this morning :-D

On Wed, 2015-04-08 at 10:16 -0600, Eric Blake wrote:
> On 04/08/2015 10:10 AM, Erik Rull wrote:
> >>
> >> My suggestion is to create a script that sends the QMP command
> >> "query-status" an then parse the result. The syntax and output is:
> >>
> >> -> { "execute": "query-status" }
> >> <- { "return": { "running": true, "singlestep": false, "status":
> >> "running" } }
> >>
> > 
> > Sounds good - I tried that - but all attempts return that the command has 
> > not
> > been found. I added the following command line snippet and the results are:
> > [...] -qmp tcp:localhost:,server,nowait [...]
> > 
> > 172.17.48.45 ~ # telnet 127.0.0.1 
> > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 2}, 
> > "package":
> > ""}, "capabilities": []}}
> 
> You HAVE to use {"execute":"qmp_capabilities"} (possibly with an
> "id":...) as your first command on the monitor, before you can issue any
> other command.  I really wish we could improve the error message:
> 
> > 
> > { "execute": "query-status" }
> > {"error": {"class": "CommandNotFound", "desc": "The command query-status 
> > has not
> > been found"}}
> 
> it would be a LOT nicer if we reported 'still in negotiation phase;
> "qmp_capabilities" expected' than a bland "CommandNotFound".  Of course,
> patches are welcome to improve the experience there!

That's an interesting point to see. I'm going to take a look on this and
submit a patch until next week, probably good to 2.4 release.

> 
> Similarly, once you are NOT in capabilities negotiation, any subsequent
> use of "qmp_capabilities" fails.  That's also something where the error
> message could be improved.
> 

-- 
Paulo Ricardo Paz Vital 
ProfitBricks GmbH




[Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

2015-04-09 Thread Thomas Huth
Current QEMU crashes when specifying an illegal model with the
"-net nic,model=xxx" option, e.g.:

 $ qemu-system-x86_64 -net nic,model=n/a
 qemu-system-x86_64: Unsupported NIC model: n/a

 Program received signal SIGSEGV, Segmentation fault.

The gdb backtrace looks like this:

0x55965fe0 in error_get_pretty (err=0x0) at util/error.c:152
152 return err->msg;
(gdb) bt
 0  0x55965fe0 in error_get_pretty (err=0x0) at util/error.c:152
 1  0x55965ffd in error_report_err (err=0x0) at util/error.c:157
 2  0x55809c90 in pci_nic_init_nofail (nd=0x55e49860 , 
rootbus=0x564409b0,
default_model=0x5598c37b "e1000", default_devaddr=0x0) at 
hw/pci/pci.c:1663
 3  0x55691e42 in pc_nic_init (isa_bus=0x56f71900, 
pci_bus=0x564409b0)
at hw/i386/pc.c:1506
 4  0x5569396b in pc_init1 (machine=0x562abbf0, pci_enabled=1, 
kvmclock_enabled=1)
at hw/i386/pc_piix.c:248
 5  0x55693d27 in pc_init_pci (machine=0x562abbf0) at 
hw/i386/pc_piix.c:310
 6  0x5572ddf5 in main (argc=3, argv=0x7fffe018, 
envp=0x7fffe038) at vl.c:4226

The problem is that pci_nic_init_nofail() does not check whether the err
parameter from pci_nic_init has been set up and thus passes a NULL pointer
to error_report_err(). Fix it by correctly checking the err parameter.

Signed-off-by: Thomas Huth 
---
 hw/pci/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6941a82..b3d5100 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
*rootbus,
 
 res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
 if (!res) {
-error_report_err(err);
+if (err) {
+error_report_err(err);
+}
 exit(1);
 }
 return res;
-- 
1.8.3.1




[Qemu-devel] Qemu binary and list of supported arches

2015-04-09 Thread Michal Privoznik
Dear list,

when trying to solve this bug [1] I've realized qemu is not reporting a
list of supported architectures for given binary. I mean, we do have
'query-target' but as I understand it it merely reports only the default
architecture. For instance for qemu-system-i386 I get 'i386', for
-x86_64 I get 'x84_64'. However, other binaries like -ppc or -ppc64 can
run other architectures like ppc, ppcle, ppc64, ppc64le and so on.

So I believe my question is, now that we have a binary capable of
running multiple architectures (and an architecture capable of being run
by several binaries) what is the best way to compute the table?

Michal

1: https://bugzilla.redhat.com/show_bug.cgi?id=1209948



Re: [Qemu-devel] [PATCH v2 1/4] target-i386: Make "level" and "xlevel" properties static

2015-04-09 Thread Igor Mammedov
On Wed,  8 Apr 2015 16:02:40 -0300
Eduardo Habkost  wrote:

> Static properties require only 1 line of code, much simpler than the
> existing code that requires writing new getters/setters.
> 
> Signed-off-by: Eduardo Habkost 
Reviewed-by: Igor Mammedov 

> ---
>  target-i386/cpu.c | 40 ++--
>  1 file changed, 2 insertions(+), 38 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 03b33cf..2bbf01d 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1618,38 +1618,6 @@ static void x86_cpuid_version_set_stepping(Object 
> *obj, Visitor *v,
>  env->cpuid_version |= value & 0xf;
>  }
>  
> -static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
> -const char *name, Error **errp)
> -{
> -X86CPU *cpu = X86_CPU(obj);
> -
> -visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
> -}
> -
> -static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
> -const char *name, Error **errp)
> -{
> -X86CPU *cpu = X86_CPU(obj);
> -
> -visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
> -}
> -
> -static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
> - const char *name, Error **errp)
> -{
> -X86CPU *cpu = X86_CPU(obj);
> -
> -visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
> -}
> -
> -static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
> - const char *name, Error **errp)
> -{
> -X86CPU *cpu = X86_CPU(obj);
> -
> -visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
> -}
> -
>  static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
>  {
>  X86CPU *cpu = X86_CPU(obj);
> @@ -2900,12 +2868,6 @@ static void x86_cpu_initfn(Object *obj)
>  object_property_add(obj, "stepping", "int",
>  x86_cpuid_version_get_stepping,
>  x86_cpuid_version_set_stepping, NULL, NULL, NULL);
> -object_property_add(obj, "level", "int",
> -x86_cpuid_get_level,
> -x86_cpuid_set_level, NULL, NULL, NULL);
> -object_property_add(obj, "xlevel", "int",
> -x86_cpuid_get_xlevel,
> -x86_cpuid_set_xlevel, NULL, NULL, NULL);
>  object_property_add_str(obj, "vendor",
>  x86_cpuid_get_vendor,
>  x86_cpuid_set_vendor, NULL);
> @@ -2998,6 +2960,8 @@ static Property x86_cpu_properties[] = {
>  DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
>  DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>  DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> +DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
> +DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
>  DEFINE_PROP_END_OF_LIST()
>  };
>  




Re: [Qemu-devel] [PATCH v5 15/21] block: Resize bitmaps on bdrv_truncate

2015-04-09 Thread Stefan Hajnoczi
On Wed, Apr 08, 2015 at 06:19:58PM -0400, John Snow wrote:
> Signed-off-by: John Snow 
> ---
>  block.c| 18 ++
>  include/qemu/hbitmap.h | 10 ++
>  util/hbitmap.c | 48 
>  3 files changed, 76 insertions(+)

Reviewed-by: Stefan Hajnoczi 


pgpDVQqLuGTg2.pgp
Description: PGP signature


Re: [Qemu-devel] [RFC PATCH 0/3] pflash_cfi01: allow reading/writing it only in secure mode

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 14:47, Peter Maydell wrote:
> On 9 April 2015 at 13:20, Paolo Bonzini  wrote:
>> This is an example of usage of attributes in a device model.  It lets
>> you block flash writes unless the CPU is in secure mode.  Enabling it
>> currently requires a -readconfig file:
>>
>> [global]
>> driver = "cfi.pflash01"
>> property = "secure"
>> value = "on"
>>
>> because the driver includes a "."; however, I plan to enable this through
>> the command line for the final version of the patches.
> 
> Are real flash devices ever wired up like this?

On x86 machines it is almost exactly like this.  I'm implementing x86
system management mode, and I'm reusing MEMTXATTRS_SECURE for it.

Recent x86 chipsets make this a run-time setting, rather than a static
setting, but the idea is the same.  It is a run-time setting (chipset
register) so that the firmware can do some initial detection of the
flash outside system management mode.  Then it writes a 1 to the
register, and finally it writes a 1 to a "lock" register so that the
first register becomes read-only.

The above scheme was actually more complicated, and allowed a race that
let you bypass the protection.  So, even more recent machines have some
additional complication, whereby flash accesses are only allowed if
_all_ processors are in system management mode.  Again, it is a run-time
setting.

QEMU emulates a slightly older chipset, which is why I'm making it a
static property.  The static property is also much harder to get wrong
and insecure by mistake.

Paolo

> I would expect boards which want to provide secure-mode
> only flash to do so by not giving any access at all to
> the device from the non-secure address space.
> 
> (Supporting multiple AddressSpaces for ARM CPUs is the
> next thing on my todo list; as well as partitioning the
> flash this would allow secure-mode-only RAM and UARTs,
> for instance.)
> 
> -- PMM
> 



Re: [Qemu-devel] [PATCH 01/14] memory: Define API for MemoryRegionOps to take attrs and return status

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 09:55, Edgar E. Iglesias  wrote:
> Did you consider using a struct here?
> e.g:
>
> typedef struct MemTxAttrs {
> unsigned int secure : 1;
> unsigned int master_id : 10;
> unsigned int etc : 1;
> } MemTxAttrs;
>
> I think you could still pass it by value and my understanding is
> that the compiler will generate similar code.

We discussed this last time round, I think. Whether structs get
passed in registers depends on the host CPU ABI/calling convention.

> I find it more readable, you ca go:
>
> attrs.secure = 1;
> attrs.master_id = 0x77;
> if (!attrs.secure)
>
> instead of:
>
> attrs |= MEMTXATTRS_SECURE
> if (!(attrs & MEMTXATTRS_SECURE))
>
> etc...
>
> Or do you see any disadvantages with this?

I prefer the traditional integer-and-bitops approach, then you
know what you're getting everywhere...

-- PMM



Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> RSDT points to other tables FADT, MADT, GTDT.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/arm/virt-acpi-build.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index a7aba75..85e84b1 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -176,6 +176,30 @@ static void acpi_dsdt_add_virtio(Aml *scope, const 
> hwaddr *mmio_addrs,
>  }
>  }
>  
> +/* RSDT */
> +static void
> +build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
> +{
> +AcpiRsdtDescriptorRev1 *rsdt;
> +size_t rsdt_len;
> +int i;
> +
> +rsdt_len = sizeof(*rsdt) + sizeof(uint32_t) * table_offsets->len;

You should use explicit brackets to be unambiguous:

   rsdt_len = sizeof(*rsdt) + (sizeof(uint32_t) * table_offsets->len);

> +rsdt = acpi_data_push(table_data, rsdt_len);
> +memcpy(rsdt->table_offset_entry, table_offsets->data,
> +   sizeof(uint32_t) * table_offsets->len);

Or perhaps split the sizes:

   const int table_data_len = (sizeof(uint32_t) * table_offsets->len);

   rsdt_len = sizeof(*rsdt) + table_data_len;
   rsdt = acpi_data_push(table_data, rsdt_len);
   memcpy(rsdt->table_offset_entry, table_offsets->data, table_data_len)

Maybe?

> +for (i = 0; i < table_offsets->len; ++i) {
> +/* rsdt->table_offset_entry to be filled by Guest linker */
> +bios_linker_loader_add_pointer(linker,
> +   ACPI_BUILD_TABLE_FILE,
> +   ACPI_BUILD_TABLE_FILE,
> +   table_data, 
> &rsdt->table_offset_entry[i],
> +   sizeof(uint32_t));

Why are these pointers always 32 bit? Can they ever be 64 bit?

> +}
> +build_header(linker, table_data,
> + (void *)rsdt, "RSDT", rsdt_len, 1);
> +}
> +
>  /* GTDT */
>  static void
>  build_gtdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> @@ -348,6 +372,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  acpi_add_table(table_offsets, tables_blob);
>  build_gtdt(tables_blob, tables->linker, guest_info);
>  
> +/* RSDT is pointed to by RSDP */
> +build_rsdt(tables_blob, tables->linker, table_offsets);
> +
>  /* Cleanup memory that's no longer used. */
>  g_array_free(table_offsets, true);
>  }

-- 
Alex Bennée



Re: [Qemu-devel] [PATCH v4 13/20] hw/acpi/aml-build: Add ToUUID macro

2015-04-09 Thread Igor Mammedov
On Fri, 3 Apr 2015 18:03:45 +0800
Shannon Zhao  wrote:

> From: Shannon Zhao 
> 
> Add ToUUID macro, this is useful for generating PCIe ACPI table.
> 
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/acpi/aml-build.c | 23 +++
>  include/hw/acpi/aml-build.h |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index bd1713c..5a94fc9 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -933,6 +933,29 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed 
> min_fixed,
>   addr_trans, len, flags);
>  }
>  
> +/*
> + * ACPI 3.0: 17.5.124 ToUUID (Convert String to UUID Macro)
> + * e.g. UUID: E5C937D0-3553-4d7a-9117-EA4D19C3434D
> + * call aml_touuid(0xE5C937D0, 0x3553, 0x4d7a, 0x9117, 0xEA4D19C3434D);
hmm,   that's definitely no string

> + */
> +Aml *aml_touuid(int32_t val1, int16_t val2, int16_t val3,
> +int16_t val4, int64_t val5)
> +{
> +int i;
> +Aml *UUID = aml_buffer();
> +
> +build_append_int_noprefix(UUID->buf, val1, 4);
> +build_append_int_noprefix(UUID->buf, val2, 2);
> +build_append_int_noprefix(UUID->buf, val3, 2);
> +build_append_int_noprefix(UUID->buf, (val4 >> 8) & 0xFF, 1);
> +build_append_int_noprefix(UUID->buf, val4 & 0xFF, 1);
> +for (i = 40; i >= 0; i -= 8) {
> +build_append_int_noprefix(UUID->buf, (val5 >> i) & 0xFF, 1);
> +}
> +
> +return UUID;
> +}
> +
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 315c729..942d986 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -209,6 +209,8 @@ Aml *aml_buffer(void);
>  Aml *aml_resource_template(void);
>  Aml *aml_field(const char *name, AmlFieldFlags flags);
>  Aml *aml_varpackage(uint32_t num_elements);
> +Aml *aml_touuid(int32_t val1, int16_t val2, int16_t val3,
> +int16_t val4, int64_t val5);
>  
>  void
>  build_header(GArray *linker, GArray *table_data,




[Qemu-devel] [PATCH 10/10] s390x/kvm: Support access register mode for KVM_S390_MEM_OP ioctl

2015-04-09 Thread Cornelia Huck
From: Alexander Yarygin 

Access register mode is one of the modes that control dynamic address
translation. In this mode the address space is specified by values of
the access registers. The effective address-space-control element is
obtained from the result of the access register translation. See
the "Access-Register Introduction" section of the chapter 5 "Program
Execution" in "Principles of Operations" for more details.

When the CPU is in AR mode, the s390_cpu_virt_mem_rw() function must
know which access register number to use for address translation.
This patch does several things:
- add new parameter 'uint8_t ar' to that function
- decode ar number from intercepted instructions
- pass the ar number to s390_cpu_virt_mem_rw(), which in turn passes it
to the KVM_S390_MEM_OP ioctl.

Signed-off-by: Alexander Yarygin 
Reviewed-by: Thomas Huth 
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/s390-pci-inst.c  | 21 +++--
 hw/s390x/s390-pci-inst.h  |  7 ---
 target-s390x/cpu.h| 30 +-
 target-s390x/ioinst.c | 42 +-
 target-s390x/kvm.c| 46 ++
 target-s390x/mmu_helper.c |  5 +++--
 6 files changed, 90 insertions(+), 61 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 08d8aa6..cac3d83 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -155,7 +155,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 return 0;
 }
 
-if (s390_cpu_virt_mem_read(cpu, env->regs[r2], buffer, sizeof(*reqh))) {
+if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer, sizeof(*reqh))) 
{
 return 0;
 }
 reqh = (ClpReqHdr *)buffer;
@@ -165,7 +165,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 return 0;
 }
 
-if (s390_cpu_virt_mem_read(cpu, env->regs[r2], buffer,
+if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer,
req_len + sizeof(*resh))) {
 return 0;
 }
@@ -180,7 +180,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 return 0;
 }
 
-if (s390_cpu_virt_mem_read(cpu, env->regs[r2], buffer,
+if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer,
req_len + res_len)) {
 return 0;
 }
@@ -277,7 +277,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 }
 
 out:
-if (s390_cpu_virt_mem_write(cpu, env->regs[r2], buffer,
+if (s390_cpu_virt_mem_write(cpu, env->regs[r2], r2, buffer,
 req_len + res_len)) {
 return 0;
 }
@@ -544,7 +544,8 @@ out:
 return 0;
 }
 
-int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr)
+int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
+uint8_t ar)
 {
 CPUS390XState *env = &cpu->env;
 S390PCIBusDevice *pbdev;
@@ -601,7 +602,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r3, uint64_t gaddr)
 return 0;
 }
 
-if (s390_cpu_virt_mem_read(cpu, gaddr, buffer, len)) {
+if (s390_cpu_virt_mem_read(cpu, gaddr, ar, buffer, len)) {
 return 0;
 }
 
@@ -694,7 +695,7 @@ static void dereg_ioat(S390PCIBusDevice *pbdev)
 pbdev->g_iota = 0;
 }
 
-int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba)
+int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
 {
 CPUS390XState *env = &cpu->env;
 uint8_t oc;
@@ -723,7 +724,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t 
fiba)
 return 0;
 }
 
-if (s390_cpu_virt_mem_read(cpu, fiba, (uint8_t *)&fib, sizeof(fib))) {
+if (s390_cpu_virt_mem_read(cpu, fiba, ar, (uint8_t *)&fib, sizeof(fib))) {
 return 0;
 }
 
@@ -769,7 +770,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t 
fiba)
 return 0;
 }
 
-int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba)
+int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
 {
 CPUS390XState *env = &cpu->env;
 uint32_t fh;
@@ -825,7 +826,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t 
fiba)
 fib.fc |= 0x10;
 }
 
-if (s390_cpu_virt_mem_write(cpu, fiba, (uint8_t *)&fib, sizeof(fib))) {
+if (s390_cpu_virt_mem_write(cpu, fiba, ar, (uint8_t *)&fib, sizeof(fib))) {
 return 0;
 }
 
diff --git a/hw/s390x/s390-pci-inst.h b/hw/s390x/s390-pci-inst.h
index 7e6c804..70fa713 100644
--- a/hw/s390x/s390-pci-inst.h
+++ b/hw/s390x/s390-pci-inst.h
@@ -281,8 +281,9 @@ int clp_service_call(S390CPU *cpu, uint8_t r2);
 int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
 int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
 int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
-int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr);
-int mpcifc

Re: [Qemu-devel] [PATCH 4/7] throttle: Add throttle group support

2015-04-09 Thread Stefan Hajnoczi
On Mon, Mar 30, 2015 at 07:19:42PM +0300, Alberto Garcia wrote:
> @@ -1941,9 +1951,11 @@ void qmp_block_set_io_throttle(const char *device, 
> int64_t bps, int64_t bps_rd,
>  aio_context_acquire(aio_context);
>  
>  if (!bs->io_limits_enabled && throttle_enabled(&cfg)) {
> -bdrv_io_limits_enable(bs);
> +bdrv_io_limits_enable(bs, has_group ? group : device);
>  } else if (bs->io_limits_enabled && !throttle_enabled(&cfg)) {
>  bdrv_io_limits_disable(bs);
> +} else if (bs->io_limits_enabled && throttle_enabled(&cfg)) {
> +bdrv_io_limits_update_group(bs, has_group ? group : device);
>  }
>  
>  if (bs->io_limits_enabled) {

The semantics are inconsistent:

1. Create drive0 with throttle group "mygroup".
2. Issue block-set-io-throttle device="drive0"

The result is that a new throttle group called "drive0" is created.  I
expected to modify the throttling configuration for drive0 (i.e.
"mygroup").

Now let's disable the throttle group:

3. Issue block-set-io-throttle with 0 values for device="drive0"

The result is that "mygroup" is changed to all 0s.

Enable should behave like disable and operate on the device's throttle
group.

Stefan


pgpIzGAvynClC.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000

2015-04-09 Thread Christopher Covington
Hi Peter,

On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell
 wrote:

>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>> index 0b192a1..3b5b875 100644
>> --- a/target-arm/translate-a64.c
>> +++ b/target-arm/translate-a64.c
>> @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>>  break;
>>  }
>>  /* HLT */
>> -unsupported_encoding(s, insn);
>> +if (imm16 == 0xf000) {
>
> You need to have the semihosting_enabled check here rather
> than in the do_interrupt code, because otherwise we won't
> behave correctly in the disabled case.

Do you have suggestions for getting semihosting_enabled defined in
translate-a64.c? I'm likely doing something dumb, but while #include
"sysemu/sysemu.h" at first seemed like the obvious approach, and
appears to work for -softmmu, I'm getting errors with that when
building -linux-user.

Thanks,
Chris



Re: [Qemu-devel] [PATCH for-2.3] configure: disable by default and warn about libxseg GPLv3 license

2015-04-09 Thread Andreas Färber
Am 09.04.2015 um 18:57 schrieb Andreas Färber:
> Am 09.04.2015 um 15:52 schrieb Stefan Hajnoczi:
>> libxseg has changed license to GPLv3.  QEMU includes GPL "v2 only" code
>> which is not compatible with GPLv3.  This means the resulting binaries
>> may not be redistributable!
>>
>> Disable Archipelago (libxseg) by default to prevent accidental license
>> violations.  Also warn if linking against libxseg is enabled to remind
>> the user.
>>
>> Note that this commit does not constitute any advice about software
>> licensing.  If you have doubts you should consult a lawyer.
> 
> :)
> 
>>
>> Cc: Chrysostomos Nanakos 
>> Suggested-by: Kevin Wolf 
>> Reported-by: Andreas Färber 
>> Signed-off-by: Stefan Hajnoczi 
[...]
> Reviewed-by: Andreas Färber 

Erm, on second thoughts the subject is missing an object and was
probably intended to say "disable Archipelago by default". ;)

> Thanks,
> Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



Re: [Qemu-devel] [PATCH 10/14] target-arm: Honour NS bits in page tables

2015-04-09 Thread Edgar E. Iglesias
On Thu, Apr 09, 2015 at 03:14:58PM +0100, Peter Maydell wrote:
> On 9 April 2015 at 12:23, Edgar E. Iglesias  wrote:
> > On Tue, Apr 07, 2015 at 09:09:56PM +0100, Peter Maydell wrote:
> >>
> >> +if (regime_is_secure(env, mmu_idx)) {
> >> +/* The page table entries may downgrade this to non-secure, but
> >> + * cannot upgrade an non-secure translation regime's attributes
> >> + * to secure.
> >> + */
> >> +*attrs = MEMTXATTRS_SECURE;
> >> +} else {
> >> +*attrs = 0;
> >> +}
> >
> >
> > Should this initialization maybe be done from some kind of secure and NS
> > per CPU attribute template?
> > What I'm trying to get to is that the machine description may want to
> > control some attributes like for example the master-id.
> >
> > Or did you have another mechanism in mind for that?
> 
> I wasn't sure whether we should be adding the tx master ID
> in target-specific code or if there was a place to OR it in
> in generic code. So I left it out for the moment, since I
> don't have an immediate use case for it, and I wanted to
> keep the series from ballooning in size too much. If we
> end up deciding to do it here then it would be a pretty
> simple change to make later.

Yes, that makes sense.

> 
> > In the hack I'm using, CPU code doesn't actually edit the attributes.
> > There are a set of attribute objects that board code sets up and the CPU
> > selects among them depending on it's state. Once the attributes hit
> > tlb_set_page_with_attrs a copy is made into the IOTLB so that
> > IOMMUs can modify the actual value in the IOTLB as they translate().
> >
> > This makes it easy for board code to for example make a non TZ
> > capable CPU look as either secure or non-secure.
> 
> I think we should probably do this sort of thing using CPU
> QOM properties that the board can set appropriately on CPU
> creation to influence CPU behaviour (eg a property to set
> a tx master ID value, with default being the cpu number).

Yes, that's one way.
It would be nice if we could end up with an interface that looks similar 
for CPUs as for DMA devices. Can be naming conventions or maybe QOMifying
the mem-attribute templates so they can be set as links or props.

Cheers,
Edgar



[Qemu-devel] [PATCH 06/10] s390x/mmu: Use access type definitions instead of magic values

2015-04-09 Thread Cornelia Huck
From: Thomas Huth 

Since there are now proper definitions for the MMU access type,
let's use them in the s390x MMU code, too, instead of the
hard-to-understand magic values.

Signed-off-by: Thomas Huth 
Reviewed-by: Jens Freimann 
Acked-by: Cornelia Huck 
Signed-off-by: Cornelia Huck 
---
 target-s390x/helper.c |  2 +-
 target-s390x/mmu_helper.c | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index f1060c2..041c9c7 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -162,7 +162,7 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr 
vaddr)
 vaddr &= 0x7fff;
 }
 
-mmu_translate(env, vaddr, 2, asc, &raddr, &prot, false);
+mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false);
 
 return raddr;
 }
diff --git a/target-s390x/mmu_helper.c b/target-s390x/mmu_helper.c
index b061c85..9b88498 100644
--- a/target-s390x/mmu_helper.c
+++ b/target-s390x/mmu_helper.c
@@ -68,7 +68,7 @@ static void trigger_prot_fault(CPUS390XState *env, 
target_ulong vaddr,
 {
 uint64_t tec;
 
-tec = vaddr | (rw == 1 ? FS_WRITE : FS_READ) | 4 | asc >> 46;
+tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | 4 | asc >> 46;
 
 DPRINTF("%s: trans_exc_code=%016" PRIx64 "\n", __func__, tec);
 
@@ -85,7 +85,7 @@ static void trigger_page_fault(CPUS390XState *env, 
target_ulong vaddr,
 int ilen = ILEN_LATER;
 uint64_t tec;
 
-tec = vaddr | (rw == 1 ? FS_WRITE : FS_READ) | asc >> 46;
+tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | asc >> 46;
 
 DPRINTF("%s: vaddr=%016" PRIx64 " bits=%d\n", __func__, vaddr, bits);
 
@@ -94,7 +94,7 @@ static void trigger_page_fault(CPUS390XState *env, 
target_ulong vaddr,
 }
 
 /* Code accesses have an undefined ilc.  */
-if (rw == 2) {
+if (rw == MMU_INST_FETCH) {
 ilen = 2;
 }
 
@@ -288,7 +288,7 @@ static int mmu_translate_asce(CPUS390XState *env, 
target_ulong vaddr,
 
 r = mmu_translate_region(env, vaddr, asc, asce, level, raddr, flags, rw,
  exc);
-if ((rw == 1) && !(*flags & PAGE_WRITE)) {
+if (rw == MMU_DATA_STORE && !(*flags & PAGE_WRITE)) {
 trigger_prot_fault(env, vaddr, asc, rw, exc);
 return -1;
 }
@@ -338,7 +338,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, 
int rw, uint64_t asc,
  * Instruction: Primary
  * Data: Secondary
  */
-if (rw == 2) {
+if (rw == MMU_INST_FETCH) {
 r = mmu_translate_asce(env, vaddr, PSW_ASC_PRIMARY, env->cregs[1],
raddr, flags, rw, exc);
 *flags &= ~(PAGE_READ | PAGE_WRITE);
-- 
2.3.5




Re: [Qemu-devel] [PATCH 0/5] fix coding style and use of typedef type

2015-04-09 Thread Gerd Hoffmann
On Do, 2015-04-09 at 02:04 +0800, Chih-Min Chao wrote:
>   bitops : fix coding style
>   ui/vnc : fix coding style
>   ui/vnc : remove 'struct' of 'typedef struct'
>   ui/console : remove 'struct' from 'typedef struct' type
>   hw/display : remove 'struct' from 'typedef QXL struct'

Reviewed-by: Gerd Hoffmann 




Re: [Qemu-devel] [PATCH v4 09/20] hw/arm/virt-acpi-build: Generate GTDT table

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> ACPI v5.1 defines GTDT for ARM devices as a place to describe timer
> related information in the system. The Arch Timer interrupts must
> be provided for GTDT.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/arm/virt-acpi-build.c| 30 ++
>  include/hw/acpi/acpi-defs.h | 37 +
>  2 files changed, 67 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index c8245ef..a7aba75 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -176,6 +176,33 @@ static void acpi_dsdt_add_virtio(Aml *scope, const 
> hwaddr *mmio_addrs,
>  }
>  }
>  
> +/* GTDT */
> +static void
> +build_gtdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +int gtdt_start = table_data->len;
> +const struct acpi_gtdt_info *info = guest_info->gtdt_info;
> +AcpiGenericTimerTable *gtdt;
> +
> +gtdt = acpi_data_push(table_data, sizeof *gtdt);
> +/* The interrupt values are the same with the device tree when adding 16 
> */
> +gtdt->secure_el1_interrupt = info->timer_s_el1;
> +gtdt->secure_el1_flags = ACPI_EDGE_SENSITIVE;
> +
> +gtdt->non_secure_el1_interrupt = info->timer_ns_el1;
> +gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
> +
> +gtdt->virtual_timer_interrupt = info->timer_virt;
> +gtdt->virtual_timer_flags = ACPI_EDGE_SENSITIVE;
> +
> +gtdt->non_secure_el2_interrupt = info->timer_ns_el2;
> +gtdt->non_secure_el2_flags = ACPI_EDGE_SENSITIVE;
> +
> +build_header(linker, table_data,
> + (void *)(table_data->data + gtdt_start), "GTDT",
> + table_data->len - gtdt_start, 1);
> +}
> +
>  /* MADT */
>  static void
>  build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
> @@ -318,6 +345,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  acpi_add_table(table_offsets, tables_blob);
>  build_madt(tables_blob, tables->linker, guest_info, &cpuinfo);
>  
> +acpi_add_table(table_offsets, tables_blob);
> +build_gtdt(tables_blob, tables->linker, guest_info);
> +
>  /* Cleanup memory that's no longer used. */
>  g_array_free(table_offsets, true);
>  }
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index e343728..ee40a5e 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -318,6 +318,43 @@ struct AcpiMadtGenericDistributor {
>  typedef struct AcpiMadtGenericDistributor AcpiMadtGenericDistributor;
>  
>  /*
> + * Generic Timer Description Table (GTDT)
> + */
> +
> +#define ACPI_GTDT_INTERRUPT_MODE(1)

(1 << 0) would be consistent 

> +#define ACPI_GTDT_INTERRUPT_POLARITY(1<<1)
> +#define ACPI_GTDT_ALWAYS_ON (1<<2)

Also spaces (n << m)

> +
> +/* Triggering */
> +
> +#define ACPI_LEVEL_SENSITIVE(uint8_t) 0x00
> +#define ACPI_EDGE_SENSITIVE (uint8_t) 0x01
> +
> +/* Polarity */
> +
> +#define ACPI_ACTIVE_HIGH(uint8_t) 0x00
> +#define ACPI_ACTIVE_LOW (uint8_t) 0x01
> +#define ACPI_ACTIVE_BOTH(uint8_t) 0x02

I'd wrap those cast defines, e.g:

#define ACPI_ACTIVE_BOTH((uint8_t) 0x02)

> +
> +struct AcpiGenericTimerTable {
> +ACPI_TABLE_HEADER_DEF
> +uint64_t counter_block_addresss;
> +uint32_t reserved;
> +uint32_t secure_el1_interrupt;
> +uint32_t secure_el1_flags;
> +uint32_t non_secure_el1_interrupt;
> +uint32_t non_secure_el1_flags;
> +uint32_t virtual_timer_interrupt;
> +uint32_t virtual_timer_flags;
> +uint32_t non_secure_el2_interrupt;
> +uint32_t non_secure_el2_flags;
> +uint64_t counter_read_block_address;
> +uint32_t platform_timer_count;
> +uint32_t platform_timer_offset;
> +} QEMU_PACKED;
> +typedef struct AcpiGenericTimerTable AcpiGenericTimerTable;
> +
> +/*
>   * HPET Description Table
>   */
>  struct Acpi20Hpet {

-- 
Alex Bennée



Re: [Qemu-devel] [PATCH v4 06/20] hw/arm/virt-acpi-build: Generation of DSDT table for virt devices

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> DSDT consists of the usual common table header plus a definition
> block in AML encoding which describes all devices in the platform.
>
> After initializing DSDT with header information the namespace is
> created which is followed by the device encodings. The devices are
> described using the Resource Template for the 32-Bit Fixed Memory
> Range and the Extended Interrupt Descriptors.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/arm/virt-acpi-build.c | 137 
> +++
>  1 file changed, 137 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 388838a..516c1d0 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -57,6 +57,139 @@
>  #define ACPI_BUILD_DPRINTF(fmt, ...)
>  #endif
>  
> +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
> +{
> +Aml *dev, *crs;
> +int i;
> +char name[5];

name, dev and crs could be declared inside the for() loop.

> +for (i = 0; i < max_cpus; i++) {
> +snprintf(name, 5, "CPU%u", i);

What happens here if you have 10 or 100 CPUs? 

> +dev = aml_device("%s", name);

Also aml_device seems to take a format string so why not simply:

 dev = aml_device("CPU%u", i)

> +aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +crs = aml_resource_template();
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +}
> +}
> +
> +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
> +   const int *uart_irq)
> +{
> +Aml *dev, *crs;
> +
> +dev = aml_device("COM0");
> +aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +crs = aml_resource_template();
> +aml_append(crs,
> +   aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
> +aml_append(crs,
> +   aml_interrupt(0x01, *uart_irq + 32));
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
> +  const int *rtc_irq)
> +{
> +Aml *dev, *crs;
> +
> +dev = aml_device("RTC0");
> +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +crs = aml_resource_template();
> +aml_append(crs,
> +   aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
> +aml_append(crs,
> +   aml_interrupt(0x01, *rtc_irq + 32));
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
> +{
> +Aml *dev, *crs;
> +hwaddr base = flash_addr[0];
> +hwaddr size = flash_addr[1];
> +
> +dev = aml_device("FLS0");
> +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +crs = aml_resource_template();
> +aml_append(crs,
> +   aml_memory32_fixed(base, size, 0x01));
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +
> +dev = aml_device("FLS1");
> +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +crs = aml_resource_template();
> +aml_append(crs,
> +   aml_memory32_fixed(base + size, size, 0x01));
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs,
> + const int *mmio_irq, int num)
> +{
> +Aml *dev, *crs;
> +hwaddr base = mmio_addrs[0];
> +hwaddr size = mmio_addrs[1];

What ensures all these hw addresses are in 32 bit space on 64 bit platforms?

> +int irq = *mmio_irq + 32;
> +int i;
> +char name[5];
> +
> +for (i = 0; i < num; i++) {
> +snprintf(name, 5, "VR%02u", i);
> +dev = aml_device("%s", name);

Again why not call aml_device directly?

> +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> +aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +
> +crs = aml_resource_template();
> +aml_append(crs,
> +   aml_memory32_fixed(base, size, 0x01));
> +aml_append(crs,
> +   aml_interrupt(0x01, irq + i));
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +aml_append(scope, dev);
> +base += size;
> +}
> +}
> +
> +/* DSDT */
> +static void
> +build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +Aml *

Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

2015-04-09 Thread Liviu Ionescu

> On 09 Apr 2015, at 10:40, Liviu Ionescu  wrote:

> I guess you either tweaked the pkg-config to find them, or you set some 
> environment variables before configure.
> ...
> can you share these details too? I want to reproduce "exactly" your 
> environment.

the reason I ask for these details is that even after adding a 
x86_64-w64-mingw32-pkg-config, the configure still differs from yours.

among the differences I spotted were:

- no -mthreads -D_POSIX=1 
- no SDL support 

another problems are related to the setup:
- "make installer" did not pack any DLL inside the setup
- the setup insists on installing in C:\Program Files (x86), although it is not 
a 32-bit application

my feeling is that you still have additional libraries that you have 
difficulties to keep track, you possibly use environment variables, and the 
procedure to generate the setup is not the one in the Makefile.


I would appreciate your help in documenting all build details.

regards,

Liviu


mkdir -p /root/build
cd /root/build
PKG_CONFIG=/root/Host/Work/qemu/gnuarmeclipse-qemu.git/gnuarmeclipse/scripts/x86_64-w64-mingw32-pkg-config
 \
/root/qemu.git/configure --cross-prefix=x86_64-w64-mingw32- 
--enable-trace-backend=stderr --extra-cflags=-Wno-missing-format-attribute 
--target-list="arm-softmmu" | tee build.txt
make
make installer

CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -mms-bitfields 
-I/usr/x86_64-w64-mingw32/include/glib-2.0 
-I/usr/x86_64-w64-mingw32/lib/glib-2.0/include -g 
QEMU_CFLAGS   -I/usr/x86_64-w64-mingw32/include/pixman-1  -m64 
-D__USE_MINGW_ANSI_STDIO=1 -DWIN32_LEAN_AND_MEAN -DWINVER=0x501 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -Wno-missing-format-attribute  -Wendif-labels 
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration 
-Wold-style-definition -Wtype-limits -fstack-protector-strong  
-I/usr/x86_64-w64-mingw32/include/libpng15
LDFLAGS   -Wl,--nxcompat -Wl,--no-seh -Wl,--dynamicbase 
-Wl,--warn-common -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   arm-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   no
GTK support   yes
VTE support   no
curses supportno
curl support  no
mingw32 support   yes
Audio drivers winwave
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC TLS support   no
VNC SASL support  no
VNC JPEG support  yes
VNC PNG support   yes
VNC WS supportno
xen support   no
brlapi supportno
bluez  supportno
Documentation yes
GUEST_BASEyes
PIE   no
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support no
Install blobs yes
KVM support   no
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportno
fdatasync no
madvise   no
posix_madvise no
sigev_thread_id   no
uuid support  no
libcap-ng support no
vhost-net support no
vhost-scsi support no
Trace backendsstderr
spice support no
rbd support   no
xfsctl supportno
nss used  no
libusbno
usb net redir no
OpenGL supportno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
seccomp support   no
coroutine backend win32
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   no
QOM debugging yes
vhdx  no
Quorumno
lzo support   no
snappy supportno
bzip2 support no
NUMA host support no





[Qemu-devel] [PATCH v2] translate-all: use glib for all page descriptor allocations

2015-04-09 Thread Emilio G. Cota
Since commit

  b7b5233a "bsd-user/mmap.c: Don't try to override g_malloc/g_free"

the exception we make here for usermode has been unnecessary.
Get rid of it.

Signed-off-by: Emilio G. Cota 
---
 translate-all.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 4d05898..c8f0e8c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -389,18 +389,6 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 void **lp;
 int i;
 
-#if defined(CONFIG_USER_ONLY)
-/* We can't use g_malloc because it may recurse into a locked mutex. */
-# define ALLOC(P, SIZE) \
-do {\
-P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,\
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
-} while (0)
-#else
-# define ALLOC(P, SIZE) \
-do { P = g_malloc0(SIZE); } while (0)
-#endif
-
 /* Level 1.  Always allocated.  */
 lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
 
@@ -412,7 +400,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(p, sizeof(void *) * V_L2_SIZE);
+p = g_malloc0(sizeof(void *) * V_L2_SIZE);
 *lp = p;
 }
 
@@ -424,12 +412,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
int alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(pd, sizeof(PageDesc) * V_L2_SIZE);
+pd = g_malloc0(sizeof(PageDesc) * V_L2_SIZE);
 *lp = pd;
 }
 
-#undef ALLOC
-
 return pd + (index & (V_L2_SIZE - 1));
 }
 
-- 
1.9.1




Re: [Qemu-devel] [PATCH 1/2] CVE-2015-1779: incrementally decode websocket frames

2015-04-09 Thread Daniel P. Berrange
On Wed, Apr 01, 2015 at 02:41:57PM +0100, Peter Maydell wrote:
> On 1 April 2015 at 14:36, Gerd Hoffmann  wrote:
> > Confirmed.  Fixes the issues I've seen in testing and looks sensible to
> > me.  Comment from Daniel would be nice, especially as I know next to
> > nothing about websockets, but he seems to be off into the easter
> > holidays already.
> >
> > So, with -rc2 waiting for this (and being late already) I think I'll
> > squash in the incremental fix and prepare a pull request even without
> > Daniels ack ...
> 
> Yes, that seems best. Given that this is a CVE fix can you
> make sure the change is called out clearly in the commit
> message so it's easy for downstreams to see which version
> of the fix they have applied? Might be worth including the
> fixup-diff in the commit message...

Yes, that fix looks correct to me too, thanks for figuring that out.

Sorry for not responding before - I've been off on paternity leave
for several weeks and only just catching up.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH v4 12/20] hw/arm/virt-acpi-build: Add PCIe info and generate MCFG table

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> Add PCIe info struct, prepare for building PCIe table.
> And generate MCFG table.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/arm/virt-acpi-build.c | 21 +
>  include/hw/arm/virt-acpi-build.h | 12 
>  2 files changed, 33 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index dd5538b..a979582 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -229,6 +229,24 @@ build_rsdt(GArray *table_data, GArray *linker, GArray 
> *table_offsets)
>   (void *)rsdt, "RSDT", rsdt_len, 1);
>  }
>  
> +static void
> +build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +AcpiTableMcfg *mcfg;
> +acpi_pcie_info *info = guest_info->pcie_info;
> +int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);

Explicit bracketing around the maths please.

> +
> +mcfg = acpi_data_push(table_data, len);
> +mcfg->allocation[0].address = cpu_to_le64(info->pcie_ecam_base);
> +
> +/* Only a single allocation so no need to play with segments */
> +mcfg->allocation[0].pci_segment = cpu_to_le16(0);
> +mcfg->allocation[0].start_bus_number = 0;
> +mcfg->allocation[0].end_bus_number = info->nr_pcie_buses - 1;
> +
> +build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1);
> +}
> +
>  /* GTDT */
>  static void
>  build_gtdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> @@ -401,6 +419,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  acpi_add_table(table_offsets, tables_blob);
>  build_gtdt(tables_blob, tables->linker, guest_info);
>  
> +acpi_add_table(table_offsets, tables_blob);
> +build_mcfg(tables_blob, tables->linker, guest_info);
> +
>  /* RSDT is pointed to by RSDP */
>  rsdt = tables_blob->len;
>  build_rsdt(tables_blob, tables->linker, table_offsets);
> diff --git a/include/hw/arm/virt-acpi-build.h 
> b/include/hw/arm/virt-acpi-build.h
> index 2780856..d534489 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -47,6 +47,17 @@ typedef struct acpi_dsdt_info {
>  const hwaddr *flash_addr;
>  } acpi_dsdt_info;
>  
> +typedef struct acpi_pcie_info {
> +const int *pcie_irq;
> +hwaddr pcie_mmio_base;
> +hwaddr pcie_mmio_size;
> +hwaddr pcie_ioport_base;
> +hwaddr pcie_ioport_size;
> +hwaddr pcie_ecam_base;
> +hwaddr pcie_ecam_size;
> +int nr_pcie_buses;
> +} acpi_pcie_info;
> +
>  typedef struct VirtGuestInfo {
>  int smp_cpus;
>  int max_cpus;
> @@ -54,6 +65,7 @@ typedef struct VirtGuestInfo {
>  acpi_madt_info *madt_info;
>  acpi_dsdt_info *dsdt_info;
>  acpi_gtdt_info *gtdt_info;
> +acpi_pcie_info *pcie_info;
>  } VirtGuestInfo;

-- 
Alex Bennée



Re: [Qemu-devel] [PULL 22/62] block: Support Archipelago as a QEMU block backend

2015-04-09 Thread Chrysostomos Nanakos

On 2015-04-09 06:48, Andreas Färber wrote:

Am 08.08.2014 um 19:39 schrieb Kevin Wolf:

From: Chrysostomos Nanakos 

VM Image on Archipelago volume is specified like this:


file.driver=archipelago,file.volume=[,file.mport=[,
file.vport=][,file.segment=]]

'archipelago' is the protocol.

'mport' is the port number on which mapperd is listening. This is 
optional
and if not specified, QEMU will make Archipelago to use the default 
port.


'vport' is the port number on which vlmcd is listening. This is 
optional
and if not specified, QEMU will make Archipelago to use the default 
port.


'segment' is the name of the shared memory segment Archipelago stack 
is using.
This is optional and if not specified, QEMU will make Archipelago to 
use the

default value, 'archipelago'.

Examples:

file.driver=archipelago,file.volume=my_vm_volume
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
file.vport=1234
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
file.vport=1234,file.segment=my_segment

Signed-off-by: Chrysostomos Nanakos 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 MAINTAINERS |   6 +
 block/Makefile.objs |   2 +
 block/archipelago.c | 787 


 configure   |  40 +++
 4 files changed, 835 insertions(+)
 create mode 100644 block/archipelago.c


Judging by configure output in v2.3.0-rc2, QEMU seems to rely on
libxseg, which is GPL-3.0+: https://github.com/grnet/libxseg

How can anyone legally build this backend then? o.O

Any chance libxseg can be relicensed to GPL-2.0+?



Hello Andreas,
indeed the license has changed a few months after submitting the patch 
for the block driver to QEMU. I have already contacted the 
administration
of GRNET which is responsible for this change asking them about the 
aforementioned problem. I am waiting for the reply and hopefully we will 
resolve this license matter.


Regards,
Chrysostomos.




[Qemu-devel] [PATCH] misc: Fix new collection of typos

2015-04-09 Thread Stefan Weil
All of them were reported by codespell.
Most typos are in comments, one is in an error message.

Signed-off-by: Stefan Weil 
---
 hw/block/virtio-blk.c |2 +-
 hw/misc/edu.c |2 +-
 hw/net/virtio-net.c   |2 +-
 hw/ppc/spapr.c|2 +-
 qga/qapi-schema.json  |2 +-
 target-s390x/mmu_helper.c |8 
 target-s390x/translate.c  |2 +-
 tests/libqos/ahci.c   |4 ++--
 8 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9546fd2..e6afe97 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -515,7 +515,7 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 type = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);
 
 /* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER
- * is an optional flag. Altough a guest should not send this flag if
+ * is an optional flag. Although a guest should not send this flag if
  * not negotiated we ignored it in the past. So keep ignoring it. */
 switch (type & ~(VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_BARRIER)) {
 case VIRTIO_BLK_T_IN:
diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index f601069..fe50b42 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -279,7 +279,7 @@ static const MemoryRegionOps edu_mmio_ops = {
 };
 
 /*
- * We purposedly use a thread, so that users are forced to wait for the status
+ * We purposely use a thread, so that users are forced to wait for the status
  * register.
  */
 static void *edu_fact_thread(void *opaque)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 59f76bc..67ab228 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1590,7 +1590,7 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
 n->max_queues = MAX(n->nic_conf.peers.queues, 1);
 if (n->max_queues * 2 + 1 > VIRTIO_PCI_QUEUE_MAX) {
 error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
-   "must be a postive integer less than %d.",
+   "must be a positive integer less than %d.",
n->max_queues, (VIRTIO_PCI_QUEUE_MAX - 1) / 2);
 virtio_cleanup(vdev);
 return;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 61ddc79..644689a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1029,7 +1029,7 @@ static int spapr_post_load(void *opaque, int version_id)
 sPAPREnvironment *spapr = (sPAPREnvironment *)opaque;
 int err = 0;
 
-/* In earlier versions, there was no seperate qdev for the PAPR
+/* In earlier versions, there was no separate qdev for the PAPR
  * RTC, so the RTC offset was stored directly in sPAPREnvironment.
  * So when migrating from those versions, poke the incoming offset
  * value into the RTC device */
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 95f49e3..5c4cd40 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -808,7 +808,7 @@
 #
 # An enumeration of memory block operation result.
 #
-# @sucess: the operation of online/offline memory block is successful.
+# @success: the operation of online/offline memory block is successful.
 # @not-found: can't find the corresponding memoryXXX directory in sysfs.
 # @operation-not-supported: for some old kernels, it does not support
 #   online or offline memory block.
diff --git a/target-s390x/mmu_helper.c b/target-s390x/mmu_helper.c
index b061c85..7baf5e9 100644
--- a/target-s390x/mmu_helper.c
+++ b/target-s390x/mmu_helper.c
@@ -303,8 +303,8 @@ static int mmu_translate_asce(CPUS390XState *env, 
target_ulong vaddr,
  * @param ascaddress space control (one of the PSW_ASC_* modes)
  * @param raddr  the translated address is stored to this pointer
  * @param flags  the PAGE_READ/WRITE/EXEC flags are stored to this pointer
- * @param exctrue = inject a program check if a fault occured
- * @return   0 if the translation was successfull, -1 if a fault occured
+ * @param exctrue = inject a program check if a fault occurred
+ * @return   0 if the translation was successful, -1 if a fault occurred
  */
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
   target_ulong *raddr, int *flags, bool exc)
@@ -436,9 +436,9 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int 
nr_pages,
  * s390_cpu_virt_mem_rw:
  * @laddr: the logical start address
  * @hostbuf:   buffer in host memory. NULL = do only checks w/o copying
- * @len:   length that should be transfered
+ * @len:   length that should be transferred
  * @is_write:  true = write, false = read
- * Returns:0 on success, non-zero if an exception occured
+ * Returns:0 on success, non-zero if an exception occurred
  *
  * Copy from/to guest memory using logical addresses. Note that we inject a
  * program interrupt in case there is an error while accessing the memory.
diff --git a/target-s39

Re: [Qemu-devel] [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 16:54, Peter Lieven wrote:
> 
> #define BM_MIGRATION_COMPAT_STATUS_BITS \
> (IDE_RETRY_DMA | IDE_RETRY_PIO | \
> IDE_RETRY_READ | IDE_RETRY_FLUSH)
> 
> Why is there no IDE_RETRY_WRITE ?

Because that's represented by none of read and flush being set. :)

> Honestly, I have not yet understood that that
> BM_MIGRATION_COMPAT_STATUS_BITS is for.

It's just for migrations while the VM is stopped due to I/O errors
(rerror=stop/werror=stop).

Paolo



Re: [Qemu-devel] seccomp breakage on arm

2015-04-09 Thread Paul Moore
On Thursday, April 09, 2015 10:21:52 AM Eduardo Otubo wrote:
> On Thu, Apr 09, 2015 at 05=01=31AM +0200, Andreas Färber wrote:
> > Hello,
> > 
> > I am seeing the following build failure on openSUSE Tumbleweed armv7l
> > with --enable-seccomp in v2.3.0-rc2:
> > 
> > [  551s] In file included from qemu-seccomp.c:16:0:
> > [  551s] /usr/include/libseccomp/seccomp.h:177:23: error: '__NR_mmap'
> > undeclared here (not in a function)
> > [  551s]  #define SCMP_SYS(x)  (__NR_##x)
> > [  551s]^
> > [  551s] qemu-seccomp.c:36:7: note: in expansion of macro 'SCMP_SYS'
> > [  551s]  { SCMP_SYS(mmap), 247 },
> > [  551s]^
> > [  551s] /usr/include/libseccomp/seccomp.h:177:23: error:
> > '__NR_getrlimit' undeclared here (not in a function)
> > [  551s]  #define SCMP_SYS(x)  (__NR_##x)
> > [  551s]^
> > [  551s] qemu-seccomp.c:57:7: note: in expansion of macro 'SCMP_SYS'
> > [  551s]  { SCMP_SYS(getrlimit), 245 },
> > [  551s]^
> > [  551s] /home/abuild/rpmbuild/BUILD/qemu-2.3.0-rc2/rules.mak:57: recipe
> > for target 'qemu-seccomp.o' failed
> > [  551s] make: *** [qemu-seccomp.o] Error 1
> > 
> > Is this a problem with libseccomp 2.2.0 / master and needs to be fixed
> > in the library? Or do we need to #ifdef some syscalls in qemu-seccomp.c?
> 
> This should be already fixed in the library as mentioned by the
> maintainer in this[0] thread. Adding Paul Moore in CC. There's also a
> bug entry on launchpad[1] for that. I provided the patch (before the
> pull reuqest) requesting for some review and testing but never heard
> back again. Also CC'ing Karl-Philipp Richter (bug owner) for some
> opinions on that as well.
> 
> Regards,
> 
> [0] http://sourceforge.net/p/libseccomp/mailman/message/32955831/
> [1] https://bugs.launchpad.net/qemu/+bug/1363641

This should be fixed with libseccomp v2.2.0; if you are still seeing problems 
using v2.2.0 let me know.

-- 
paul moore
security @ redhat




Re: [Qemu-devel] [PATCH 01/14] memory: Define API for MemoryRegionOps to take attrs and return status

2015-04-09 Thread Edgar E. Iglesias
On Tue, Apr 07, 2015 at 09:09:47PM +0100, Peter Maydell wrote:
> Define an API so that devices can register MemoryRegionOps whose read
> and write callback functions are passed an arbitrary pointer to some
> transaction attributes and can return a success-or-failure status code.
> This will allow us to model devices which:
>  * behave differently for ARM Secure/NonSecure memory accesses
>  * behave differently for privileged/unprivileged accesses
>  * may return a transaction failure (causing a guest exception)
>for erroneous accesses
> 
> This patch defines the new API and plumbs the attributes parameter through
> to the memory.c public level functions io_mem_read() and io_mem_write(),
> where it is currently dummied out.
> 
> The success/failure response indication is also propagated out to
> io_mem_read() and io_mem_write(), which retain the old-style
> boolean true-for-error return.
> 
> Signed-off-by: Peter Maydell 
> ---
>  include/exec/memattrs.h |  34 
>  include/exec/memory.h   |  22 +
>  memory.c| 207 
> 
>  3 files changed, 196 insertions(+), 67 deletions(-)
>  create mode 100644 include/exec/memattrs.h
> 
> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> new file mode 100644
> index 000..b8d7808
> --- /dev/null
> +++ b/include/exec/memattrs.h
> @@ -0,0 +1,34 @@
> +/*
> + * Memory transaction attributes
> + *
> + * Copyright (c) 2015 Linaro Limited.
> + *
> + * Authors:
> + *  Peter Maydell 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef MEMATTRS_H
> +#define MEMATTRS_H
> +
> +/* Every memory transaction has associated with it a set of
> + * attributes. Some of these are generic (such as the ID of
> + * the bus master); some are specific to a particular kind of
> + * bus (such as the ARM Secure/NonSecure bit). We define them
> + * all as non-overlapping bits in a single integer to avoid
> + * confusion if different parts of QEMU used the same bit for
> + * different semantics.
> + */
> +typedef uint64_t MemTxAttrs;

Hi,

Did you consider using a struct here?
e.g:

typedef struct MemTxAttrs {
unsigned int secure : 1;
unsigned int master_id : 10;
unsigned int etc : 1;
} MemTxAttrs;

I think you could still pass it by value and my understanding is
that the compiler will generate similar code.

I find it more readable, you ca go:

attrs.secure = 1;
attrs.master_id = 0x77;
if (!attrs.secure)

instead of: 

attrs |= MEMTXATTRS_SECURE
if (!(attrs & MEMTXATTRS_SECURE))

etc...

Or do you see any disadvantages with this?



> +
> +/* Bus masters which don't specify any attributes will get this,
> + * which has all attribute bits clear except the topmost one
> + * (so that we can distinguish "all attributes deliberately clear"
> + * from "didn't specify" if necessary).
> + */
> +#define MEMTXATTRS_UNSPECIFIED (1ULL << 63)
> +
> +#endif
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 06ffa1d..703d9e5 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -28,6 +28,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "exec/hwaddr.h"
>  #endif
> +#include "exec/memattrs.h"
>  #include "qemu/queue.h"
>  #include "qemu/int128.h"
>  #include "qemu/notify.h"
> @@ -68,6 +69,16 @@ struct IOMMUTLBEntry {
>  IOMMUAccessFlags perm;
>  };
>  
> +/* New-style MMIO accessors can indicate that the transaction failed.
> + * A zero (MEMTX_OK) response means success; anything else is a failure
> + * of some kind. The memory subsystem will bitwise-OR together results
> + * if it is synthesizing an operation from multiple smaller accesses.
> + */
> +#define MEMTX_OK 0
> +#define MEMTX_ERROR (1U << 0) /* device returned an error */
> +#define MEMTX_DECODE_ERROR  (1U << 1) /* nothing at that address */
> +typedef uint32_t MemTxResult;
> +
>  /*
>   * Memory region callbacks
>   */
> @@ -84,6 +95,17 @@ struct MemoryRegionOps {
>uint64_t data,
>unsigned size);
>  
> +MemTxResult (*read_with_attrs)(void *opaque,
> +   hwaddr addr,
> +   uint64_t *data,
> +   unsigned size,
> +   MemTxAttrs attrs);
> +MemTxResult (*write_with_attrs)(void *opaque,
> +hwaddr addr,
> +uint64_t data,
> +unsigned size,
> +MemTxAttrs attrs);
> +
>  enum device_endian endianness;
>  /* Guest-visible constraints: */
>  struct {
> diff --git a/memory.c b/memory.c
> index ee3f2a8..9bb5674 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -368,57 +368,84 @@ static void adjust_endianness(MemoryRegion *mr, 
> uint64_t *data, unsigned size)
>  }
>  }
>  
> 

Re: [Qemu-devel] [PATCH] qemu-config: Accept empty option values

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 20:50, Eduardo Habkost wrote:
> On Thu, Apr 09, 2015 at 02:11:11PM +0200, Paolo Bonzini wrote:
>> On 08/04/2015 20:16, Eduardo Habkost wrote:
>>> Currently it is impossible to set an option in a config file to an empty
>>> string, because the parser matches only lines containing non-empty
>>> strings between double-quotes.
>>>
>>> As sscanf() "[" conversion specifier only matches non-empty strings, add
>>> a special case for empty strings.
>>>
>>> Signed-off-by: Eduardo Habkost 
>>> ---
>>>  util/qemu-config.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/util/qemu-config.c b/util/qemu-config.c
>>> index 2d32ce7..9f9577d 100644
>>> --- a/util/qemu-config.c
>>> +++ b/util/qemu-config.c
>>> @@ -413,7 +413,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, 
>>> const char *fname)
>>>  opts = qemu_opts_create(list, NULL, 0, &error_abort);
>>>  continue;
>>>  }
>>> -if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
>>> +if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
>>> +(value[0] = '\0', sscanf(line, " %63s = \"\"", arg) == 1)) {
>>>  /* arg = value */
>>>  if (opts == NULL) {
>>>  error_report("no group defined");
>>>
>>
>> Acked-by: Paolo Bonzini 
> 
> Thanks! Whose tree this should go through?

Acked-by generally means that you're welcome to take it through yours.

My "misc" tree hosts non-trivial patches outside well-defined
maintainership area, but it's less work to carry patches only from
people who don't send pull requests.  Maybe it's not the case of this
patch, but using your own tree also means that it's simpler for you to
sort out the dependencies.

Paolo



Re: [Qemu-devel] seccomp breakage on arm

2015-04-09 Thread Andreas Färber
Am 09.04.2015 um 11:10 schrieb Paul Moore:
> On Thursday, April 09, 2015 10:21:52 AM Eduardo Otubo wrote:
>> On Thu, Apr 09, 2015 at 05=01=31AM +0200, Andreas Färber wrote:
>>> Hello,
>>>
>>> I am seeing the following build failure on openSUSE Tumbleweed armv7l
>>> with --enable-seccomp in v2.3.0-rc2:
>>>
>>> [  551s] In file included from qemu-seccomp.c:16:0:
>>> [  551s] /usr/include/libseccomp/seccomp.h:177:23: error: '__NR_mmap'
>>> undeclared here (not in a function)
>>> [  551s]  #define SCMP_SYS(x)  (__NR_##x)
>>> [  551s]^
>>> [  551s] qemu-seccomp.c:36:7: note: in expansion of macro 'SCMP_SYS'
>>> [  551s]  { SCMP_SYS(mmap), 247 },
>>> [  551s]^
>>> [  551s] /usr/include/libseccomp/seccomp.h:177:23: error:
>>> '__NR_getrlimit' undeclared here (not in a function)
>>> [  551s]  #define SCMP_SYS(x)  (__NR_##x)
>>> [  551s]^
>>> [  551s] qemu-seccomp.c:57:7: note: in expansion of macro 'SCMP_SYS'
>>> [  551s]  { SCMP_SYS(getrlimit), 245 },
>>> [  551s]^
>>> [  551s] /home/abuild/rpmbuild/BUILD/qemu-2.3.0-rc2/rules.mak:57: recipe
>>> for target 'qemu-seccomp.o' failed
>>> [  551s] make: *** [qemu-seccomp.o] Error 1
>>>
>>> Is this a problem with libseccomp 2.2.0 / master and needs to be fixed
>>> in the library? Or do we need to #ifdef some syscalls in qemu-seccomp.c?
>>
>> This should be already fixed in the library as mentioned by the
>> maintainer in this[0] thread. Adding Paul Moore in CC. There's also a
>> bug entry on launchpad[1] for that. I provided the patch (before the
>> pull reuqest) requesting for some review and testing but never heard
>> back again. Also CC'ing Karl-Philipp Richter (bug owner) for some
>> opinions on that as well.
>>
>> Regards,
>>
>> [0] http://sourceforge.net/p/libseccomp/mailman/message/32955831/
>> [1] https://bugs.launchpad.net/qemu/+bug/1363641
> 
> This should be fixed with libseccomp v2.2.0; if you are still seeing problems 
> using v2.2.0 let me know.

This *is* with libseccomp v2.2.0, as mentioned above, and I had checked
that there were no related changes beyond v2.2.0 on your master branch.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



Re: [Qemu-devel] [PATCH 2/7] throttle: Add throttle group infrastructure

2015-04-09 Thread Stefan Hajnoczi
On Mon, Mar 30, 2015 at 07:19:40PM +0300, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> ---
>  block/Makefile.objs |   1 +
>  block/throttle-groups.c | 261 
> 
>  include/block/block_int.h   |   1 +
>  include/block/throttle-groups.h |  39 ++
>  4 files changed, 302 insertions(+)
>  create mode 100644 block/throttle-groups.c
>  create mode 100644 include/block/throttle-groups.h

Reviewed-by: Stefan Hajnoczi 


pgpO6xFMzHn8G.pgp
Description: PGP signature


[Qemu-devel] [PULL 0/4] Block patches

2015-04-09 Thread Stefan Hajnoczi
The following changes since commit 5a24f20a7208a58fb80d78ca0521bba6f4d7b145:

  Merge remote-tracking branch 
'remotes/mjt/tags/pull-trivial-patches-2015-04-04' into staging (2015-04-07 
14:33:46 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 05b685fbabb7fdcab72cb42b27db916fd74b2265:

  block/iscsi: handle zero events from iscsi_which_events (2015-04-09 10:31:45 
+0100)





Kevin Wolf (1):
  qcow2: Fix header update with overridden backing file

Paolo Bonzini (2):
  virtio-blk: correctly dirty guest memory
  aio: strengthen memory barriers for bottom half scheduling

Peter Lieven (1):
  block/iscsi: handle zero events from iscsi_which_events

 async.c | 28 ++--
 block/iscsi.c   | 33 +++---
 block/qcow2.c   | 29 ++---
 block/qcow2.h   |  6 +++
 hw/block/dataplane/virtio-blk.c |  3 +-
 hw/block/virtio-blk.c   | 13 +-
 include/hw/virtio/virtio-blk.h  |  1 +
 tests/qemu-iotests/130  | 95 +
 tests/qemu-iotests/130.out  | 43 +++
 tests/qemu-iotests/group|  1 +
 10 files changed, 220 insertions(+), 32 deletions(-)
 create mode 100755 tests/qemu-iotests/130
 create mode 100644 tests/qemu-iotests/130.out

-- 
2.1.0




Re: [Qemu-devel] [Qemu-block] [RFC] Intermediate block mirroring

2015-04-09 Thread Stefan Hajnoczi
On Thu, Apr 02, 2015 at 03:28:57PM +0200, Alberto Garcia wrote:
> Hi,
> 
> I'm interested in adding the possibility to mirror an intermediate
> node in a disk image chain, but I would like to have some feedback
> before sending any patches.
> 
> The goal would be to convert this:
> 
>[A] -> [B] -> [C] -> [D]
> 
> into this:
> 
>[A] -> [B] -> [X] -> [D]
> 
> where [D] is the active image and [X] would be a copy of [C]. The
> latter would be unlinked from the chain.
> 
> A use case would be to move disk images across different storage
> backends.

The simple solution to that problem is:

Assumption: backing files are read only.  (True in most cases.)

1. Copy the backing files using cp(1) or another method.
2. Issue QMP 'change-backing-file' command so that [D] uses [X] instead
   of [C].

So it can be done today already.

Note that the management tool needs to enforce the assumption since QEMU
cannot know whether other programs or QEMU instances are modifying one
of the backing files.

Stefan


pgp1VQs12mbmf.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 1/2] kvm: introduce kvm_arch_msi_data_to_gsi

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 17:20, Eric Auger wrote:
> On ARM the MSI data corresponds to the shared peripheral interrupt (SPI)
> ID. This latter equals to the SPI index + 32. to retrieve the SPI index,
> matching the gsi, an architecture specific function is introduced.
> 
> Signed-off-by: Eric Auger 
> ---
>  include/sysemu/kvm.h | 2 ++
>  kvm-all.c| 2 +-
>  target-arm/kvm.c | 5 +
>  target-i386/kvm.c| 5 +
>  target-mips/kvm.c| 5 +
>  target-ppc/kvm.c | 5 +
>  target-s390x/kvm.c   | 5 +

Please abort on i386/mips/s390x, as they do not set
kvm_gsi_direct_mapping(); ok with this change.

Paolo

>  7 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 197e6c0..ebd8b17 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -286,6 +286,8 @@ void kvm_arch_init_irq_routing(KVMState *s);
>  int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>   uint64_t address, uint32_t data);
>  
> +int kvm_arch_msi_data_to_gsi(uint32_t data);
> +
>  int kvm_set_irq(KVMState *s, int irq, int level);
>  int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
>  
> diff --git a/kvm-all.c b/kvm-all.c
> index dd44f8c..3e9675e 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1228,7 +1228,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage 
> msg)
>  int virq;
>  
>  if (kvm_gsi_direct_mapping()) {
> -return msg.data & 0x;
> +return kvm_arch_msi_data_to_gsi(msg.data);
>  }
>  
>  if (!kvm_gsi_routing_enabled()) {
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index fdd9ba3..05d5634 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -598,3 +598,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
> *route,
>  {
>  return 0;
>  }
> +
> +int kvm_arch_msi_data_to_gsi(uint32_t data)
> +{
> +return (data - 32) & 0x;
> +}
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 41d09e5..7c648e7 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2764,3 +2764,8 @@ int kvm_arch_fixup_msi_route(struct 
> kvm_irq_routing_entry *route,
>  {
>  return 0;
>  }
> +
> +int kvm_arch_msi_data_to_gsi(uint32_t data)
> +{
> +return data & 0x;
> +}
> diff --git a/target-mips/kvm.c b/target-mips/kvm.c
> index 4d1f7ea..e53e059 100644
> --- a/target-mips/kvm.c
> +++ b/target-mips/kvm.c
> @@ -694,3 +694,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
> *route,
>  {
>  return 0;
>  }
> +
> +int kvm_arch_msi_data_to_gsi(uint32_t data)
> +{
> +return data & 0x;
> +}
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 12328a4..53569f6 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2408,3 +2408,8 @@ int kvm_arch_fixup_msi_route(struct 
> kvm_irq_routing_entry *route,
>  {
>  return 0;
>  }
> +
> +int kvm_arch_msi_data_to_gsi(uint32_t data)
> +{
> +return data & 0x;
> +}
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b48c643..6509aff 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -1940,3 +1940,8 @@ int kvm_arch_fixup_msi_route(struct 
> kvm_irq_routing_entry *route,
>  route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
>  return 0;
>  }
> +
> +int kvm_arch_msi_data_to_gsi(uint32_t data)
> +{
> +return data & 0x;
> +}
> 



Re: [Qemu-devel] [PATCH v4 06/20] hw/arm/virt-acpi-build: Generation of DSDT table for virt devices

2015-04-09 Thread Igor Mammedov
On Thu, 09 Apr 2015 10:51:33 +0100
Alex Bennée  wrote:

> 
> Shannon Zhao  writes:
> 
> > From: Shannon Zhao 
> >
> > DSDT consists of the usual common table header plus a definition
> > block in AML encoding which describes all devices in the platform.
> >
> > After initializing DSDT with header information the namespace is
> > created which is followed by the device encodings. The devices are
> > described using the Resource Template for the 32-Bit Fixed Memory
> > Range and the Extended Interrupt Descriptors.
> >
> > Signed-off-by: Shannon Zhao 
> > Signed-off-by: Shannon Zhao 
> > ---
> >  hw/arm/virt-acpi-build.c | 137 
> > +++
> >  1 file changed, 137 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 388838a..516c1d0 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -57,6 +57,139 @@
> >  #define ACPI_BUILD_DPRINTF(fmt, ...)
I wonder if all this DPRINTF crap should be replaced by tracepoints.
Oo at least it shouldn't be used in new code.

> >  #endif
> >  
> > +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
> > +{
> > +Aml *dev, *crs;
> > +int i;
> > +char name[5];
> 
> name, dev and crs could be declared inside the for() loop.
> 
> > +for (i = 0; i < max_cpus; i++) {
> > +snprintf(name, 5, "CPU%u", i);
> 
> What happens here if you have 10 or 100 CPUs? 
> 
> > +dev = aml_device("%s", name);
> 
> Also aml_device seems to take a format string so why not simply:
> 
>  dev = aml_device("CPU%u", i)
On top of that, ACPI name is limited to 4 characters so
CPU%u will allow to specify only 10 cpus, to support more
shrink CPU part and do something like:

Scope(CPUS) {
Device(C000) {}
...
Device(CFFF) {}
}

that would give us upto max 4096 CPU,
I have on TODO list to convert bitmap based x86 cpu hotplug to
memory hotplug based interface so that we could easily
switch to more CPUs when KVM starts support it.
And for ARM there is no point to have/maintain 2 interfaces
as we would have to do on x86.

> 
> > +aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
> > +aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> > +crs = aml_resource_template();
> > +aml_append(dev, aml_name_decl("_CRS", crs));
> > +aml_append(scope, dev);
> > +}
> > +}
> > +
> > +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
> > +   const int *uart_irq)
> > +{
> > +Aml *dev, *crs;
> > +
> > +dev = aml_device("COM0");
> > +aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +crs = aml_resource_template();
> > +aml_append(crs,
> > +   aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
uart_addr[0], uart_addr[1] doesn't tell anything about what they are.
and doing array of it when it's not dynamic sized is confusing not to
mention passing addresses and sizes in it.
The same goes for the rest of foo_addr[] used in this patch/series,
if it's _addr then it shouldn't contain sizes.

how about:
 acpi_dsdt_add_uart(scope, uint32_t uart_addr, uint32_t uart_size, ...)

 
> > +aml_append(crs,
> > +   aml_interrupt(0x01, *uart_irq + 32));
> > +aml_append(dev, aml_name_decl("_CRS", crs));
> > +aml_append(scope, dev);
> > +}
> > +
> > +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
> > +  const int *rtc_irq)
> > +{
> > +Aml *dev, *crs;
> > +
> > +dev = aml_device("RTC0");
> > +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +crs = aml_resource_template();
> > +aml_append(crs,
> > +   aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
> > +aml_append(crs,
> > +   aml_interrupt(0x01, *rtc_irq + 32));
> > +aml_append(dev, aml_name_decl("_CRS", crs));
> > +aml_append(scope, dev);
> > +}
> > +
> > +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
> > +{
> > +Aml *dev, *crs;
> > +hwaddr base = flash_addr[0];
> > +hwaddr size = flash_addr[1];
> > +
> > +dev = aml_device("FLS0");
> > +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> > +aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +crs = aml_resource_template();
> > +aml_append(crs,
> > +   aml_memory32_fixed(base, size, 0x01));
join it with line above it if it doesn't exceed 80 chr
same goes for other places.

> > +aml_append(dev, aml_name_decl("_CRS", crs));
> > +aml_append(scope, dev);
> > +
> > +dev = aml_device("FLS1");
> > +aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> > +aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> 

[Qemu-devel] [PULL 1/4] qcow2: Fix header update with overridden backing file

2015-04-09 Thread Stefan Hajnoczi
From: Kevin Wolf 

In recent qemu versions, it is possible to override the backing file
name and format that is stored in the image file with values given at
runtime. In such cases, the temporary override could end up in the
image header if the qcow2 header was updated, while obviously correct
behaviour would be to leave the on-disk backing file path/format
unchanged.

Fix this and add a test case for it.

Reported-by: Michael Tokarev 
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Message-id: 1428411796-2852-1-git-send-email-kw...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2.c  | 29 ++
 block/qcow2.h  |  6 +++
 tests/qemu-iotests/130 | 95 ++
 tests/qemu-iotests/130.out | 43 +
 tests/qemu-iotests/group   |  1 +
 5 files changed, 167 insertions(+), 7 deletions(-)
 create mode 100755 tests/qemu-iotests/130
 create mode 100644 tests/qemu-iotests/130.out

diff --git a/block/qcow2.c b/block/qcow2.c
index 32bdf75..316a8db 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -140,6 +140,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, 
uint64_t start_offset,
 return 3;
 }
 bs->backing_format[ext.len] = '\0';
+s->image_backing_format = g_strdup(bs->backing_format);
 #ifdef DEBUG_EXT
 printf("Qcow2: Got format extension %s\n", bs->backing_format);
 #endif
@@ -884,6 +885,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto fail;
 }
 bs->backing_file[len] = '\0';
+s->image_backing_file = g_strdup(bs->backing_file);
 }
 
 /* Internal snapshots */
@@ -1457,6 +1459,9 @@ static void qcow2_close(BlockDriverState *bs)
 g_free(s->unknown_header_fields);
 cleanup_unknown_header_ext(bs);
 
+g_free(s->image_backing_file);
+g_free(s->image_backing_format);
+
 g_free(s->cluster_cache);
 qemu_vfree(s->cluster_data);
 qcow2_refcount_close(bs);
@@ -1622,9 +1627,10 @@ int qcow2_update_header(BlockDriverState *bs)
 }
 
 /* Backing file format header extension */
-if (*bs->backing_format) {
+if (s->image_backing_format) {
 ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BACKING_FORMAT,
- bs->backing_format, strlen(bs->backing_format),
+ s->image_backing_format,
+ strlen(s->image_backing_format),
  buflen);
 if (ret < 0) {
 goto fail;
@@ -1682,8 +1688,8 @@ int qcow2_update_header(BlockDriverState *bs)
 buflen -= ret;
 
 /* Backing file name */
-if (*bs->backing_file) {
-size_t backing_file_len = strlen(bs->backing_file);
+if (s->image_backing_file) {
+size_t backing_file_len = strlen(s->image_backing_file);
 
 if (buflen < backing_file_len) {
 ret = -ENOSPC;
@@ -1691,7 +1697,7 @@ int qcow2_update_header(BlockDriverState *bs)
 }
 
 /* Using strncpy is ok here, since buf is not NUL-terminated. */
-strncpy(buf, bs->backing_file, buflen);
+strncpy(buf, s->image_backing_file, buflen);
 
 header->backing_file_offset = cpu_to_be64(buf - ((char*) header));
 header->backing_file_size   = cpu_to_be32(backing_file_len);
@@ -1712,9 +1718,17 @@ fail:
 static int qcow2_change_backing_file(BlockDriverState *bs,
 const char *backing_file, const char *backing_fmt)
 {
+BDRVQcowState *s = bs->opaque;
+
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_file ?: "");
 pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_fmt ?: "");
 
+g_free(s->image_backing_file);
+g_free(s->image_backing_format);
+
+s->image_backing_file = backing_file ? g_strdup(bs->backing_file) : NULL;
+s->image_backing_format = backing_fmt ? g_strdup(bs->backing_format) : 
NULL;
+
 return qcow2_update_header(bs);
 }
 
@@ -2751,8 +2765,9 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 }
 
 if (backing_file || backing_format) {
-ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,
-backing_format ?: bs->backing_format);
+ret = qcow2_change_backing_file(bs,
+backing_file ?: s->image_backing_file,
+backing_format ?: s->image_backing_format);
 if (ret < 0) {
 return ret;
 }
diff --git a/block/qcow2.h b/block/qcow2.h
index aa6d367..422b825 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -283,6 +283,12 @@ typedef struct BDRVQcowState {
 QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;
 QTAILQ_HEAD (, Qcow2DiscardRegion) discards;
 bool cache_discards;
+
+/* Backing file path and format as stored in the image (this is not the
+ * effective path/format, which may be the result of a runtime opt

Re: [Qemu-devel] [PULL 22/62] block: Support Archipelago as a QEMU block backend

2015-04-09 Thread Chrysostomos Nanakos

On 2015-04-09 17:05, Stefan Hajnoczi wrote:
On Thu, Apr 9, 2015 at 1:48 PM, Chrysostomos Nanakos 
 wrote:

On 2015-04-09 06:48, Andreas Färber wrote:


Am 08.08.2014 um 19:39 schrieb Kevin Wolf:


From: Chrysostomos Nanakos 

VM Image on Archipelago volume is specified like this:




file.driver=archipelago,file.volume=[,file.mport=[,
file.vport=][,file.segment=]]

'archipelago' is the protocol.

'mport' is the port number on which mapperd is listening. This is
optional
and if not specified, QEMU will make Archipelago to use the 
default port.


'vport' is the port number on which vlmcd is listening. This is 
optional
and if not specified, QEMU will make Archipelago to use the 
default port.


'segment' is the name of the shared memory segment Archipelago 
stack is

using.
This is optional and if not specified, QEMU will make Archipelago 
to use

the
default value, 'archipelago'.

Examples:

file.driver=archipelago,file.volume=my_vm_volume
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
file.vport=1234
file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
file.vport=1234,file.segment=my_segment

Signed-off-by: Chrysostomos Nanakos 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 MAINTAINERS |   6 +
 block/Makefile.objs |   2 +
 block/archipelago.c | 787

 configure   |  40 +++
 4 files changed, 835 insertions(+)
 create mode 100644 block/archipelago.c



Judging by configure output in v2.3.0-rc2, QEMU seems to rely on
libxseg, which is GPL-3.0+: https://github.com/grnet/libxseg

How can anyone legally build this backend then? o.O

Any chance libxseg can be relicensed to GPL-2.0+?



Hello Andreas,
indeed the license has changed a few months after submitting the 
patch for
the block driver to QEMU. I have already contacted the 
administration

of GRNET which is responsible for this change asking them about the
aforementioned problem. I am waiting for the reply and hopefully we 
will

resolve this license matter.


In the meantime I've sent a patch to disable Archipelago by default 
in

./configure ("[PATCH for-2.3] configure: disable by default and warn
about libxseg GPLv3 license").  You are CCed on the email.

The intention is to reduce the chance of users accidentally compiling
binaries with license problems.  Archipelago support is still
available but it must be explicitly enabled using ./configure
--enable-archipelago.

Stefan


For the moment I totally agree with the proposed patch.

Regards,
Chrysostomos.




Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

2015-04-09 Thread Stefan Weil

Am 09.04.2015 um 16:07 schrieb Liviu Ionescu

On 09 Apr 2015, at 10:40, Liviu Ionescu  wrote:
I guess you either tweaked the pkg-config to find them, or you set some 
environment variables before configure.
...
can you share these details too? I want to reproduce "exactly" your environment.



Here is the template for all cross pkg-configs:
http://repo.or.cz/w/qemu/ar7.git/blob_plain/HEAD:/scripts/cross-pkg-config

See also http://patchwork.ozlabs.org/patch/114242/

Maybe I should resend that patch.


the reason I ask for these details is that even after adding a 
x86_64-w64-mingw32-pkg-config, the configure still differs from yours.

among the differences I spotted were:

- no -mthreads -D_POSIX=1
- no SDL support



I think -mthreads is essential (needed for thread local storage),
and -D_POSIX=1 is also needed for 64 bit builds.

Maybe you can try my QEMU fork from http://repo.or.cz/w/qemu/ar7.git
and see whether it works for you. My binaries are based on that code.
If it works, we have to look which patches are missing in the official
QEMU version.

SDL comes from a locally cross built libSDL. I never had time to put
it into a Debian package. Maybe it is better for your tests to build
without SDL support, because the SDL init code redirects stdin and
stdout to files which complicates things even when you run QEMU
with SDL disabled.
.


  


another problems are related to the setup:
- "make installer" did not pack any DLL inside the setup


It has a built-in magic: all DLL files from $(SRC_PATH)/dll/w64 are
taken for the 64 bit build (w32 for 32 bit build).

Get a list of all needed DLL files either by try and error or by using
tools and copy them (or make symbolic links) to that directory.
Here are the files for 32 bit for my configuration:

dll/w32/freetype6.dll
dll/w32/pdcurses.dll
dll/w32/libgio-2.0-0.dll
dll/w32/libfontconfig-1.dll
dll/w32/libpangoft2-1.0-0.dll
dll/w32/libpangocairo-1.0-0.dll
dll/w32/libgtk-win32-2.0-0.dll
dll/w32/libatk-1.0-0.dll
dll/w32/libpangowin32-1.0-0.dll
dll/w32/intl.dll
dll/w32/libpango-1.0-0.dll
dll/w32/libgcc_s_sjlj-1.dll
dll/w32/libgdk_pixbuf-2.0-0.dll
dll/w32/libglib-2.0-0.dll
dll/w32/libssp-0.dll
dll/w32/zlib1.dll
dll/w32/libgmodule-2.0-0.dll
dll/w32/libcairo-2.dll
dll/w32/SDL.dll
dll/w32/libgthread-2.0-0.dll
dll/w32/libexpat-1.dll
dll/w32/libpng14-14.dll
dll/w32/libgobject-2.0-0.dll
dll/w32/libgdk-win32-2.0-0.dll
dll/w32/libstdc++-6.dll


- the setup insists on installing in C:\Program Files (x86), although it is not 
a 32-bit application

my feeling is that you still have additional libraries that you have 
difficulties to keep track, you possibly use environment variables, and the 
procedure to generate the setup is not the one in the Makefile.


There is an nsis macro W64 which switches the program directory (see 
qemu.nsi).

Are you using the packages from Debian (here those for Wheezy):

ii  nsis 2.46-7amd64Nullsoft Scriptable 
Install System (modified for Debian)
ii  nsis-common 2.46-7all  Nullsoft 
Scriptable Install System stubs and plugins



Regards
Stefan




[Qemu-devel] [PATCH] translate-all: use g_malloc0 for all page descriptor allocations

2015-04-09 Thread Emilio G. Cota
Since commit

  b7b5233a "bsd-user/mmap.c: Don't try to override g_malloc/g_free"

the exception we make here for usermode has been unnecessary.
Get rid of it.

Signed-off-by: Emilio G. Cota 
---
 translate-all.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 11763c6..3a6d116 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -389,18 +389,6 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 void **lp;
 int i;
 
-#if defined(CONFIG_USER_ONLY)
-/* We can't use g_malloc because it may recurse into a locked mutex. */
-# define ALLOC(P, SIZE) \
-do {\
-P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,\
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
-} while (0)
-#else
-# define ALLOC(P, SIZE) \
-do { P = g_malloc0(SIZE); } while (0)
-#endif
-
 /* Level 1.  Always allocated.  */
 lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
 
@@ -412,7 +400,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(p, sizeof(void *) * V_L2_SIZE);
+p = g_malloc0(sizeof(void *) * V_L2_SIZE);
 *lp = p;
 }
 
@@ -424,12 +412,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
int alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(pd, sizeof(PageDesc) * V_L2_SIZE);
+pd = g_malloc0(sizeof(PageDesc) * V_L2_SIZE);
 *lp = pd;
 }
 
-#undef ALLOC
-
 return pd + (index & (V_L2_SIZE - 1));
 }
 
-- 
1.9.1




Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes

2015-04-09 Thread Edgar E. Iglesias
On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote:
> Make address_space_rw take transaction attributes, rather
> than always using the 'unspecified' attributes.

Reviewed-by: Edgar E. Iglesias 

I guess that we eventually will need to convert the dma_
functions?



> 
> Signed-off-by: Peter Maydell 
> ---
>  dma-helpers.c|  3 ++-
>  exec.c   | 51 
> ++--
>  hw/mips/mips_jazz.c  |  6 --
>  hw/pci-host/prep.c   |  6 --
>  include/exec/memory.h| 31 ++---
>  include/sysemu/dma.h |  3 ++-
>  ioport.c | 16 +--
>  kvm-all.c|  3 ++-
>  scripts/coverity-model.c |  8 +---
>  9 files changed, 77 insertions(+), 50 deletions(-)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 6918572..33b1983 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -28,7 +28,8 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, 
> uint8_t c, dma_addr_t len)
>  memset(fillbuf, c, FILLBUF_SIZE);
>  while (len > 0) {
>  l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
> -error |= address_space_rw(as, addr, fillbuf, l, true);
> +error |= address_space_rw(as, addr, MEMTXATTRS_UNSPECIFIED,
> +  fillbuf, l, true);
>  len -= l;
>  addr += l;
>  }
> diff --git a/exec.c b/exec.c
> index 7e53f52..29a12fa 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1946,13 +1946,16 @@ static MemTxResult subpage_read(void *opaque, hwaddr 
> addr, uint64_t *data,
>  {
>  subpage_t *subpage = opaque;
>  uint8_t buf[8];
> +MemTxResult res;
>  
>  #if defined(DEBUG_SUBPAGE)
>  printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__,
> subpage, len, addr);
>  #endif
> -if (address_space_read(subpage->as, addr + subpage->base, buf, len)) {
> -return MEMTX_DECODE_ERROR;
> +res = address_space_read(subpage->as, addr + subpage->base,
> + attrs, buf, len);
> +if (res) {
> +return res;
>  }
>  switch (len) {
>  case 1:
> @@ -1999,10 +2002,8 @@ static MemTxResult subpage_write(void *opaque, hwaddr 
> addr,
>  default:
>  abort();
>  }
> -if (address_space_write(subpage->as, addr + subpage->base, buf, len)) {
> -return MEMTX_DECODE_ERROR;
> -}
> -return MEMTX_OK;
> +return address_space_write(subpage->as, addr + subpage->base,
> +   attrs, buf, len);
>  }
>  
>  static bool subpage_accepts(void *opaque, hwaddr addr,
> @@ -2313,8 +2314,8 @@ static int memory_access_size(MemoryRegion *mr, 
> unsigned l, hwaddr addr)
>  return l;
>  }
>  
> -bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
> -  int len, bool is_write)
> +MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> + uint8_t *buf, int len, bool is_write)
>  {
>  hwaddr l;
>  uint8_t *ptr;
> @@ -2322,7 +2323,6 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, 
> uint8_t *buf,
>  hwaddr addr1;
>  MemoryRegion *mr;
>  MemTxResult result = MEMTX_OK;
> -MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
>  
>  while (len > 0) {
>  l = len;
> @@ -2406,22 +2406,24 @@ bool address_space_rw(AddressSpace *as, hwaddr addr, 
> uint8_t *buf,
>  return result;
>  }
>  
> -bool address_space_write(AddressSpace *as, hwaddr addr,
> - const uint8_t *buf, int len)
> +MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs 
> attrs,
> +const uint8_t *buf, int len)
>  {
> -return address_space_rw(as, addr, (uint8_t *)buf, len, true);
> +return address_space_rw(as, addr, attrs, (uint8_t *)buf, len, true);
>  }
>  
> -bool address_space_read(AddressSpace *as, hwaddr addr, uint8_t *buf, int len)
> +MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs 
> attrs,
> +   uint8_t *buf, int len)
>  {
> -return address_space_rw(as, addr, buf, len, false);
> +return address_space_rw(as, addr, attrs, buf, len, false);
>  }
>  
>  
>  void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
>  int len, int is_write)
>  {
> -address_space_rw(&address_space_memory, addr, buf, len, is_write);
> +address_space_rw(&address_space_memory, addr, MEMTXATTRS_UNSPECIFIED,
> + buf, len, is_write);
>  }
>  
>  enum write_rom_type {
> @@ -2592,7 +2594,8 @@ void *address_space_map(AddressSpace *as,
>  memory_region_ref(mr);
>  bounce.mr = mr;
>  if (!is_write) {
> -address_space_read(as, addr, bounce.buffer, l);
> +address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED,
> +   bounce.buffer, l);
>  }
>  
>  *plen = l;
> @@ -2645

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add myself as NUMA code maintainer

2015-04-09 Thread Paolo Bonzini


On 08/04/2015 18:53, Eduardo Habkost wrote:
> The "srat" and "numa" keywords will help get_maintainer.pl catch
> NUMA-related code in other files too.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  MAINTAINERS | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d7e9ba2..978e714 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -905,6 +905,15 @@ F: nbd.*
>  F: qemu-nbd.c
>  T: git git://github.com/bonzini/qemu.git nbd-next
>  
> +NUMA
> +M: Eduardo Habkost 
> +S: Maintained
> +F: numa.c
> +F: include/sysemu/numa.h
> +K: numa|NUMA
> +K: srat|SRAT
> +T: git git://github.com/ehabkost/qemu.git numa
> +
>  QAPI
>  M: Luiz Capitulino 
>  M: Michael Roth 
> 

Acked-by: Paolo Bonzini 



Re: [Qemu-devel] SIGTERM signal to qemu-kvm process

2015-04-09 Thread Jatin Davey

On 4/8/2015 9:06 PM, Kevin Wolf wrote:

Am 08.04.2015 um 07:07 hat Jatin Davey geschrieben:

I am using QEMU 0.12.1 as the hypervisor in my RHEL installation of 6.5.

I wanted to know if there are any side-effects with respect to VM image
corruption when i use SIGTERM signal to kill a qemu-kvm process which
effectively stops my VM running on the host.

Appreciate if you can provide me some valuable information in this regard.

Sending SIGTERM is equivalent to sending a 'quit' monitor command. This
is a clean shutdown as far as qemu is concerned, but of course it isn't
for your guest OS.

If your guest behaves correctly, you may lose data that wasn't flushed
to disk yet, but you should not get any corruption. The image file
structure itself (i.e. what 'qemu-img check' checks) should be fully
consistent.

Sending SIGKILL is a bit worse (just like a host crash/power failure),
because then even the image file may be inconsistent. But again, if the
guest behaves correct (and assuming there are no qemu bugs and you're
not using cache=unsafe), even in this case the result should never be
corruption, but only loss of data that wasn't flushed to disk yet.

Kevin

Thanks Kevin.

Appreciate your prompt response in this regard.

Thanks
Jatin



Re: [Qemu-devel] [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1

2015-04-09 Thread Peter Lieven

Am 07.04.2015 um 21:13 schrieb John Snow:



On 04/07/2015 03:02 PM, Peter Lieven wrote:

Am 07.04.2015 um 20:56 schrieb John Snow:



On 04/07/2015 02:44 PM, Peter Lieven wrote:

Am 07.04.2015 um 17:29 schrieb Dr. David Alan Gilbert:

* Peter Lieven (p...@kamp.de) wrote:

Hi David,

Am 07.04.2015 um 10:43 schrieb Dr. David Alan Gilbert:

Any particular workload or reproducer?

Workload is almost zero. I try to figure out if there is a way to trigger it.

Maybe playing a role: Machine type is -M pc1.2 and we set -kvmclock as
CPU flag since kvmclock seemed to be quite buggy in 2.6.16...

Exact cmdline is:
/usr/bin/qemu-2.2.1  -enable-kvm  -M pc-1.2 -nodefaults -netdev type=tap,id=guest2,script=no,downscript=no,ifname=tap2 -device e1000,netdev=guest2,mac=52:54:00:ff:00:65 -drive 
format=raw,file=iscsi://172.21.200.53/iqn.2001-05.com.equallogic:4-52aed6-88a7e99a4-d9e00040fdc509a3-XXX-hd0/0,if=ide,cache=writeback,aio=native -serial null  -parallel null  -m 1024 -smp 2,sockets=1,cores=2,threads=1  -monitor 
tcp:0:4003,server,nowait -vnc :3 -qmp tcp:0:3003,server,nowait -name 'XXX' -boot order=c,once=dc,menu=off  -drive index=2,media=cdrom,if=ide,cache=unsafe,aio=native,readonly=on -k de  -incoming tcp:0:5003  -pidfile /var/run/qemu/vm-146.pid  
-mem-path /hugepages -mem-prealloc  -rtc base=utc -usb -usbdevice tablet -no-hpet -vga cirrus  -cpu qemu64,-kvmclock


Exact kernel is:
2.6.16.46-0.12-smp (i think this is SLES10 or sth.)

The machine does not hang. It seems just I/O is hanging. So you can type at the 
console or ping the system, but no longer login.

Thank you,
Peter

Interesting observation: Migrating the vServer again seems to fix to problem 
(at least in one case I could test just now).

2.6.8-24-smp is also affected.

How often does it fail - you say 'sometimes' - is it a 1/10 or a 1/1000 ?

Its more often than 1/10 I would say.

OK, that's not too bad - it's the 1/1000 that are really nasty to find.
In your setup, how easy would it be for you to try :
  with either 2.1 or current head?
  with a newer machine-type?
  without the cdrom?


Its all possible. I can clone the system and try everything on my test systems. 
I hope
it reproduces there.

Has the cdrom the power of taking down the bus?

Peter



I don't know if CDROM could stall the entire bus, but I suspect the reason for asking is this: dgilbert and I had tracked down a problem previously where during migration, outstanding requests being handled by the ATAPI code can get lost during 
migration if, for instance, the user has only prepared the command (via bmdma) but has not yet written to the register to activate the command yet.


That sounds like it could be related.



So if something like this happens:

- User writes to the ATA registers to program a command
- Migration occurs
- User writes to the BMDMA register to initiate the command

We can lose some of the state and data of the request. David had checked in a 
workaround for at least ATAPI that simply coaxes the guest OS into trying the 
command again to unstick it.


Do you have the commit for me?



http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg01109.html



I think we determined last time that we couldn't fix this problem without changing the migration format, so we opted not to do it for 2.3. We had also only noticed it with ATAPI drives, not HDDs, so a proper fix got kicked down the road since we 
thought the workaround was sufficient.


Maybe normally we use virtio nowadays and maybe the new kernel implementation 
(libata /dev/sdX) can't get locked? What I do not understand is how a second 
migration can unlock from this state?



IIRC our success rate with reproducing it was something on the order of 1/50, 
too.

If you can reproduce it without a CDROM but using the BMDMA interface, that's a 
good data point. If you can't reproduce it using the ISA interface, that's a 
phenomenal data point and implicates BMDMA pretty heavily.


To be 100% sure we are talking about the same? How would I use the ISA and how 
would I use the BMDMA interface?

Thanks,
Peter



BMDMA is the PCI HBA for IDE, I think it's the default for most machines that 
aren't using the AHCI HBA.

To get ISA, try launching with the machine "isapc" which will force it, or add the device 
manually, it's named "isa-ide".
The BMDMA PCI device is just named "ide".


Unfortunately, the BIOS can't boot if I specify device isa-ide.

Peter



Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container

2015-04-09 Thread Alexey Kardashevskiy

On 04/09/2015 04:43 PM, David Gibson wrote:

On Wed, Apr 08, 2015 at 01:45:19PM +1000, Alexey Kardashevskiy wrote:

On 04/08/2015 12:01 PM, David Gibson wrote:

On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:

This enables multiple IOMMU groups in one VFIO container which means
that multiple devices from different groups can share the same IOMMU
table (or tables if DDW).

This removes a group id from vfio_container_ioctl(). The kernel support
is required for this; if the host kernel does not have the support,
it will allow only one group per container. The PHB's "iommuid" property
is ignored.

This adds a sanity check that there is just one VFIO container per
PHB address space.

Signed-off-by: Alexey Kardashevskiy 


[snip]

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b012620..99e1900 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
  close(vbasedev->fd);
  }

-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
+static int vfio_container_do_ioctl(AddressSpace *as,
 int req, void *param)
  {
-VFIOGroup *group;
  VFIOContainer *container;
-int ret = -1;
+int ret;
+VFIOAddressSpace *space;

-group = vfio_get_group(groupid, as);
-if (!group) {
-error_report("vfio: group %d not registered", groupid);
-return ret;
-}
+space = vfio_get_address_space(as);
+container = QLIST_FIRST(&space->containers);


So getting the container handle from the address space, rather than
the group id certainly makes more sense to me.


-container = group->container;
-if (group->container) {
+if (!container) {
+error_report("vfio: container is not set");
+return -1;
+} else if (QLIST_NEXT(container, next)) {
+error_report("vfio: multiple containers per PHB are not supported");
+return -1;


But if only one PHB per address space is possible, why is the
containers field a list in the first place?



Historically the list was added in 3df3e0a5872 (the patch of yours
:) ).


Heh.


In theory we could implement spapr-pci-bridge (derived from pci-bridge) with
isolation capability (i.e. its own LIOBN/DMA window), in this case there
could be multiple containers per PHB address space. Other archs could want
multiple containers for some other reason. It would help me a lot if you
remembered why you kept the list at the first place :)


Ok, I've looked over the patch and it has jogged my memory a bit.  So
the dumb answer is that it's because the per address-space list was
replacing a global list of containers

The more useful answer is that I think it was because I was
anticipating the possibility of working around the
one-group-per-container limit by allowing a single VFIOAddressSpace in
qemu to be backed by several containers, whose mappings would be kept
in sync from the userspace side by duplicating all mappings.

Anyway, I think that means the right way to implement this is by
duplicating the ioctl() across all the attached containers, rather
than picking just one.


Right. I will do that.



For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
code to a separate file") before this one, do s/vfio_container_do_ioctl/
vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes
sense?


That sounds fine, though I don't see that it really addresses the
question here.


You are right, it does not. I won't do it in this patchset then. Thanks.










+} else {
  ret = ioctl(container->fd, req, param);
  if (ret < 0) {
  error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
@@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, 
int32_t groupid,
  }
  }

-vfio_put_group(group);
-
  return ret;
  }

-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+int vfio_container_ioctl(AddressSpace *as,
   int req, void *param)
  {
  /* We allow only certain ioctls to the container */
@@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
  return -1;
  }

-return vfio_container_do_ioctl(as, groupid, req, param);
+return vfio_container_do_ioctl(as, req, param);
  }
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index 0b26cd8..76b5744 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -3,7 +3,7 @@

  #include "qemu/typedefs.h"

-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+extern int vfio_container_ioctl(AddressSpace *as,
  int req, void *param);

  #endif










--
Alexey



[Qemu-devel] [PULL 2/4] virtio-blk: correctly dirty guest memory

2015-04-09 Thread Stefan Hajnoczi
From: Paolo Bonzini 

After qemu_iovec_destroy, the QEMUIOVector's size is zeroed and
the zero size ultimately is used to compute virtqueue_push's len
argument.  Therefore, reads from virtio-blk devices did not
migrate their results correctly.  (Writes were okay).

Save the size in virtio_blk_handle_request, and use it when the request
is completed.

Based on a patch by Wen Congyang.

Signed-off-by: Wen Congyang 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Stefan Hajnoczi 
Tested-by: Li Zhijian 
Message-id: 1427997044-392-1-git-send-email-pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/dataplane/virtio-blk.c |  3 +--
 hw/block/virtio-blk.c   | 13 -
 include/hw/virtio/virtio-blk.h  |  1 +
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index cd41478..3db139b 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -77,8 +77,7 @@ static void complete_request_vring(VirtIOBlockReq *req, 
unsigned char status)
 VirtIOBlockDataPlane *s = req->dev->dataplane;
 stb_p(&req->in->status, status);
 
-vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem,
-   req->qiov.size + sizeof(*req->in));
+vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem, req->in_len);
 
 /* Suppress notification to guest by BH and its scheduled
  * flag because requests are completed as a batch after io
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 000c38d..9546fd2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -33,6 +33,7 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
 VirtIOBlockReq *req = g_slice_new(VirtIOBlockReq);
 req->dev = s;
 req->qiov.size = 0;
+req->in_len = 0;
 req->next = NULL;
 req->mr_next = NULL;
 return req;
@@ -54,7 +55,7 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req,
 trace_virtio_blk_req_complete(req, status);
 
 stb_p(&req->in->status, status);
-virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
+virtqueue_push(s->vq, &req->elem, req->in_len);
 virtio_notify(vdev, s->vq);
 }
 
@@ -102,6 +103,14 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 if (ret) {
 int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);
 bool is_read = !(p & VIRTIO_BLK_T_OUT);
+/* Note that memory may be dirtied on read failure.  If the
+ * virtio request is not completed here, as is the case for
+ * BLOCK_ERROR_ACTION_STOP, the memory may not be copied
+ * correctly during live migration.  While this is ugly,
+ * it is acceptable because the device is free to write to
+ * the memory until the request is completed (which will
+ * happen on the other side of the migration).
+ */
 if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
 continue;
 }
@@ -496,6 +505,8 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 exit(1);
 }
 
+/* We always touch the last byte, so just see how big in_iov is.  */
+req->in_len = iov_size(in_iov, in_num);
 req->in = (void *)in_iov[in_num - 1].iov_base
   + in_iov[in_num - 1].iov_len
   - sizeof(struct virtio_blk_inhdr);
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index b3ffcd9..6bf5905 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -67,6 +67,7 @@ typedef struct VirtIOBlockReq {
 struct virtio_blk_inhdr *in;
 struct virtio_blk_outhdr out;
 QEMUIOVector qiov;
+size_t in_len;
 struct VirtIOBlockReq *next;
 struct VirtIOBlockReq *mr_next;
 BlockAcctCookie acct;
-- 
2.1.0




[Qemu-devel] [PATCH v3] translate-all: use glib for all page descriptor allocations

2015-04-09 Thread Emilio G. Cota
Since commit

  b7b5233a "bsd-user/mmap.c: Don't try to override g_malloc/g_free"

the exception we make here for usermode has been unnecessary.
Get rid of it.

Signed-off-by: Emilio G. Cota 
---
 translate-all.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 4d05898..acf792c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -389,18 +389,6 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 void **lp;
 int i;
 
-#if defined(CONFIG_USER_ONLY)
-/* We can't use g_malloc because it may recurse into a locked mutex. */
-# define ALLOC(P, SIZE) \
-do {\
-P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,\
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
-} while (0)
-#else
-# define ALLOC(P, SIZE) \
-do { P = g_malloc0(SIZE); } while (0)
-#endif
-
 /* Level 1.  Always allocated.  */
 lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
 
@@ -412,7 +400,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int 
alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(p, sizeof(void *) * V_L2_SIZE);
+p = g_new0(void *, V_L2_SIZE);
 *lp = p;
 }
 
@@ -424,12 +412,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
int alloc)
 if (!alloc) {
 return NULL;
 }
-ALLOC(pd, sizeof(PageDesc) * V_L2_SIZE);
+pd = g_new0(PageDesc, V_L2_SIZE);
 *lp = pd;
 }
 
-#undef ALLOC
-
 return pd + (index & (V_L2_SIZE - 1));
 }
 
-- 
1.9.1




Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 12:43, Peter Maydell wrote:
> > At this point, some memory barriers, basically.
>
> So what distinguishes a device that needs the memory barriers
> and does its accesses via dma_* from a device that doesn't and
> uses address_space_* or ld/st*_phys ? (Or for that matter a
> non-device that does memory accesses...)

I don't know exactly, I didn't follow the discussion very much back
then.  The memory barriers were fixing PPC bugs; PCI devices definitely
need them.

Paolo



Re: [Qemu-devel] [RFC PATCH 0/3] pflash_cfi01: allow reading/writing it only in secure mode

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 18:10, Laszlo Ersek wrote:
> In OVMF, the reset vector and the SEC phase code run from (read-only)
> flash. SEC decompresses everything else to RAM. Also, SEC does not
> access read-write flash (the varstore) at all.
> 
> The above is a specialty of OVMF. In ArmVirtualizationQemu (aka AAVMF),
> two further module types run from flash, after SEC: PEI_CORE, and some
> PEIMs (ie. the PEI phase comes into the picture). During PEI, read-only
> access to the varstore should be supported.

Read-only access should always be fine (though with a tweak to these
patches, and slower---because it exits to QEMU---if another CPU is
looking at the flash in MMIO mode).  The problem is execution.

But on x86 flash should never be accessed by multiple CPUs at the same
time, unless all of them know that the flash is in ROM mode.

As I understand it, on ARM secure (EL3) and non-secure (EL<3) modes have
effectively different address spaces.  Therefore, one EL3 CPU could put
the flash in MMIO mode for programming, while another EL1 CPU could be
reading from the flash in ROM mode.  In QEMU, this could be implemented
with two memory regions and per-CPU address spaces.  These patches
should not get in the way, but they would not be useful.

Thanks,

Paolo

> ... I'm providing the above as "standalone facts", neither as
> confirmation nor as disproof for what you wrote. I don't know enough to
> combine these edk2 bits with what you wrote myself, but my hope is that
> *you* can maybe combine them, if I point them out. :)




Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

2015-04-09 Thread Igor Mammedov
On Thu, 09 Apr 2015 13:50:52 +0100
Alex Bennée  wrote:

> 
> Shannon Zhao  writes:
> 
> > From: Shannon Zhao 
> >
> > RSDT points to other tables FADT, MADT, GTDT.
> >
> > Signed-off-by: Shannon Zhao 
> > Signed-off-by: Shannon Zhao 
> > ---
> >  hw/arm/virt-acpi-build.c | 27 +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index a7aba75..85e84b1 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -176,6 +176,30 @@ static void acpi_dsdt_add_virtio(Aml *scope, const 
> > hwaddr *mmio_addrs,
> >  }
> >  }
> >  
> > +/* RSDT */
> > +static void
> > +build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
this function looks like exact copy of x86 impl.,
could you reuse that?

> > +{
> > +AcpiRsdtDescriptorRev1 *rsdt;
> > +size_t rsdt_len;
> > +int i;
> > +
> > +rsdt_len = sizeof(*rsdt) + sizeof(uint32_t) * table_offsets->len;
> 
> You should use explicit brackets to be unambiguous:
> 
>rsdt_len = sizeof(*rsdt) + (sizeof(uint32_t) * table_offsets->len);
> 
> > +rsdt = acpi_data_push(table_data, rsdt_len);
> > +memcpy(rsdt->table_offset_entry, table_offsets->data,
> > +   sizeof(uint32_t) * table_offsets->len);
> 
> Or perhaps split the sizes:
> 
>const int table_data_len = (sizeof(uint32_t) * table_offsets->len);
> 
>rsdt_len = sizeof(*rsdt) + table_data_len;
>rsdt = acpi_data_push(table_data, rsdt_len);
>memcpy(rsdt->table_offset_entry, table_offsets->data, table_data_len)
> 
> Maybe?
> 
> > +for (i = 0; i < table_offsets->len; ++i) {
> > +/* rsdt->table_offset_entry to be filled by Guest linker */
> > +bios_linker_loader_add_pointer(linker,
> > +   ACPI_BUILD_TABLE_FILE,
> > +   ACPI_BUILD_TABLE_FILE,
> > +   table_data, 
> > &rsdt->table_offset_entry[i],
> > +   sizeof(uint32_t));
> 
> Why are these pointers always 32 bit? Can they ever be 64 bit?
Laszlo, can you confirm that UEFI puts APCI tables below 4G address space?


> 
> > +}
> > +build_header(linker, table_data,
> > + (void *)rsdt, "RSDT", rsdt_len, 1);
> > +}
> > +
> >  /* GTDT */
> >  static void
> >  build_gtdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> > @@ -348,6 +372,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> > AcpiBuildTables *tables)
> >  acpi_add_table(table_offsets, tables_blob);
> >  build_gtdt(tables_blob, tables->linker, guest_info);
> >  
> > +/* RSDT is pointed to by RSDP */
> > +build_rsdt(tables_blob, tables->linker, table_offsets);
> > +
> >  /* Cleanup memory that's no longer used. */
> >  g_array_free(table_offsets, true);
> >  }
> 




[Qemu-devel] [PATCH 2/3] pflash_cfi01: change to new-style MMIO accessors

2015-04-09 Thread Paolo Bonzini
This is a required step to implement read_with_attrs and write_with_attrs.

Signed-off-by: Paolo Bonzini 
---
 hw/block/pflash_cfi01.c | 96 ++---
 1 file changed, 10 insertions(+), 86 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 7507a15..0b3667a 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -650,101 +650,25 @@ static void pflash_write(pflash_t *pfl, hwaddr offset,
 }
 
 
-static uint32_t pflash_readb_be(void *opaque, hwaddr addr)
-{
-return pflash_read(opaque, addr, 1, 1);
-}
-
-static uint32_t pflash_readb_le(void *opaque, hwaddr addr)
-{
-return pflash_read(opaque, addr, 1, 0);
-}
-
-static uint32_t pflash_readw_be(void *opaque, hwaddr addr)
+static uint64_t pflash_mem_read(void *opaque, hwaddr addr, unsigned len)
 {
 pflash_t *pfl = opaque;
+bool be = !!(pfl->features & (1 << PFLASH_BE));
 
-return pflash_read(pfl, addr, 2, 1);
+return pflash_read(pfl, addr, len, be);
 }
 
-static uint32_t pflash_readw_le(void *opaque, hwaddr addr)
+static void pflash_mem_write(void *opaque, hwaddr addr, uint64_t value, 
unsigned len)
 {
 pflash_t *pfl = opaque;
+bool be = !!(pfl->features & (1 << PFLASH_BE));
 
-return pflash_read(pfl, addr, 2, 0);
+pflash_write(pfl, addr, value, len, be);
 }
 
-static uint32_t pflash_readl_be(void *opaque, hwaddr addr)
-{
-pflash_t *pfl = opaque;
-
-return pflash_read(pfl, addr, 4, 1);
-}
-
-static uint32_t pflash_readl_le(void *opaque, hwaddr addr)
-{
-pflash_t *pfl = opaque;
-
-return pflash_read(pfl, addr, 4, 0);
-}
-
-static void pflash_writeb_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_write(opaque, addr, value, 1, 1);
-}
-
-static void pflash_writeb_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_write(opaque, addr, value, 1, 0);
-}
-
-static void pflash_writew_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_t *pfl = opaque;
-
-pflash_write(pfl, addr, value, 2, 1);
-}
-
-static void pflash_writew_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_t *pfl = opaque;
-
-pflash_write(pfl, addr, value, 2, 0);
-}
-
-static void pflash_writel_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_t *pfl = opaque;
-
-pflash_write(pfl, addr, value, 4, 1);
-}
-
-static void pflash_writel_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
-pflash_t *pfl = opaque;
-
-pflash_write(pfl, addr, value, 4, 0);
-}
-
-static const MemoryRegionOps pflash_cfi01_ops_be = {
-.old_mmio = {
-.read = { pflash_readb_be, pflash_readw_be, pflash_readl_be, },
-.write = { pflash_writeb_be, pflash_writew_be, pflash_writel_be, },
-},
-.endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static const MemoryRegionOps pflash_cfi01_ops_le = {
-.old_mmio = {
-.read = { pflash_readb_le, pflash_readw_le, pflash_readl_le, },
-.write = { pflash_writeb_le, pflash_writew_le, pflash_writel_le, },
-},
+static const MemoryRegionOps pflash_cfi01_ops = {
+.read = pflash_mem_read,
+.write = pflash_mem_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -775,7 +699,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 
 memory_region_init_rom_device(
 &pfl->mem, OBJECT(dev),
-pfl->features & (1 << PFLASH_BE) ? &pflash_cfi01_ops_be : 
&pflash_cfi01_ops_le,
+&pflash_cfi01_ops,
 pfl,
 pfl->name, total_len, &local_err);
 if (local_err) {
-- 
2.3.5





Re: [Qemu-devel] [PATCH v4 04/20] hw/acpi/aml-build: Add aml_memory32_fixed() term

2015-04-09 Thread Alex Bennée

Michael S. Tsirkin  writes:

> On Wed, Apr 08, 2015 at 03:54:45PM +0100, Alex Bennée wrote:
>> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> > index 8d01959..fefe7c7 100644
>> > --- a/hw/acpi/aml-build.c
>> > +++ b/hw/acpi/aml-build.c
>> > @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml 
>> > *arg2, Aml *arg3, Aml *arg4)
>> >  return var;
>> >  }
>> >  
>> > +/*
>> > + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
>> > + */
>> > +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
>> > +{
>> > +Aml *var = aml_alloc();
>> 
>> This is more aimed at the ACPI maintainers but I wonder if there should
>> be an aml_alloc_sized that pre-allocates the GArray? Otherwise we spend
>> a lot of time realloc'ing while building these entries up. Or even a
>> varidac build_append_bytes?
>
> Can you show measureable VM boot speedup from this?
> If not, it's not worth bothering with.

Fair enough, this is a start-up thing and I guess you could probably
only measure a difference in a highly targeted pathaological test case.

-- 
Alex Bennée



Re: [Qemu-devel] [PATCH 05/14] exec.c: Convert subpage memory ops to _with_attrs

2015-04-09 Thread Edgar E. Iglesias
On Tue, Apr 07, 2015 at 09:09:51PM +0100, Peter Maydell wrote:
> Convert the subpage memory ops to _with_attrs; this will allow
> us to pass the attributes through to the underlying access
> functions.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  exec.c | 33 +
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 52e7a2a..7e53f52 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1941,8 +1941,8 @@ static const MemoryRegionOps watch_mem_ops = {
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static uint64_t subpage_read(void *opaque, hwaddr addr,
> - unsigned len)
> +static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
> +unsigned len, MemTxAttrs attrs)
>  {
>  subpage_t *subpage = opaque;
>  uint8_t buf[8];
> @@ -1951,23 +1951,29 @@ static uint64_t subpage_read(void *opaque, hwaddr 
> addr,
>  printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__,
> subpage, len, addr);
>  #endif
> -address_space_read(subpage->as, addr + subpage->base, buf, len);
> +if (address_space_read(subpage->as, addr + subpage->base, buf, len)) {
> +return MEMTX_DECODE_ERROR;
> +}
>  switch (len) {
>  case 1:
> -return ldub_p(buf);
> +*data = ldub_p(buf);
> +return MEMTX_OK;
>  case 2:
> -return lduw_p(buf);
> +*data = lduw_p(buf);
> +return MEMTX_OK;
>  case 4:
> -return ldl_p(buf);
> +*data = ldl_p(buf);
> +return MEMTX_OK;
>  case 8:
> -return ldq_p(buf);
> +*data = ldq_p(buf);
> +return MEMTX_OK;
>  default:
>  abort();
>  }
>  }
>  
> -static void subpage_write(void *opaque, hwaddr addr,
> -  uint64_t value, unsigned len)
> +static MemTxResult subpage_write(void *opaque, hwaddr addr,
> + uint64_t value, unsigned len, MemTxAttrs 
> attrs)
>  {
>  subpage_t *subpage = opaque;
>  uint8_t buf[8];
> @@ -1993,7 +1999,10 @@ static void subpage_write(void *opaque, hwaddr addr,
>  default:
>  abort();
>  }
> -address_space_write(subpage->as, addr + subpage->base, buf, len);
> +if (address_space_write(subpage->as, addr + subpage->base, buf, len)) {
> +return MEMTX_DECODE_ERROR;
> +}
> +return MEMTX_OK;
>  }
>  
>  static bool subpage_accepts(void *opaque, hwaddr addr,
> @@ -2010,8 +2019,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
>  }
>  
>  static const MemoryRegionOps subpage_ops = {
> -.read = subpage_read,
> -.write = subpage_write,
> +.read_with_attrs = subpage_read,
> +.write_with_attrs = subpage_write,
>  .impl.min_access_size = 1,
>  .impl.max_access_size = 8,
>  .valid.min_access_size = 1,
> -- 
> 1.9.1
> 



Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

2015-04-09 Thread Laszlo Ersek
On 04/09/15 18:03, Peter Maydell wrote:
> On 9 April 2015 at 17:00, Laszlo Ersek  wrote:
>> On 04/09/15 15:59, Peter Maydell wrote:
>>> On 9 April 2015 at 14:51, Igor Mammedov  wrote:
 On Thu, 9 Apr 2015 14:27:58 +0100
 Peter Maydell  wrote:

> On 9 April 2015 at 14:17, Igor Mammedov  wrote:
>> On Thu, 09 Apr 2015 13:50:52 +0100
>> Alex Bennée  wrote:
>>
>>>
>>> Shannon Zhao  writes:
 +for (i = 0; i < table_offsets->len; ++i) {
 +/* rsdt->table_offset_entry to be filled by Guest linker */
 +bios_linker_loader_add_pointer(linker,
 +   ACPI_BUILD_TABLE_FILE,
 +   ACPI_BUILD_TABLE_FILE,
 +   table_data, 
 &rsdt->table_offset_entry[i],
 +   sizeof(uint32_t));
>>>
>>> Why are these pointers always 32 bit? Can they ever be 64 bit?
>> Laszlo, can you confirm that UEFI puts APCI tables below 4G address
>> space?
>>
>> I confirmed that before, in the v2 discussion:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/316670/focus=317560
>>
>> But in fact the RSDT / XSDT that QEMU exports for UEFI doesn't matter.
> 
> If this table is never used, presumably we should just
> not generate it at all, then?

Unfortunately, this is not the case. In order to identify ACPI tables *at all* 
in UEFI, I need "relocate pointer" commands for pointers that point to those 
tables. And those pointers must *reside* somewhere, in some blob.

Here's how the "relocate pointer" command is defined in edk2 
(OvmfPkg/AcpiPlatformDxe/QemuLoader.h):

//
// QemuLoaderCmdAddPointer: the bytes at
// [PointerOffset..PointerOffset+PointerSize) in the file PointerFile contain a
// relative pointer (an offset) into PointeeFile. Increment the relative
// pointer's value by the base address of where PointeeFile's contents have
// been placed (when QemuLoaderCmdAllocate has been executed for PointeeFile).
//
typedef struct {
  UINT8  PointerFile[QEMU_LOADER_FNAME_SIZE]; // NUL-terminated
  UINT8  PointeeFile[QEMU_LOADER_FNAME_SIZE]; // NUL-terminated
  UINT32 PointerOffset;
  UINT8  PointerSize; // one of 1, 2, 4, 8
} QEMU_LOADER_ADD_POINTER;

In the qemu tree, see COMMAND_ADD_POINTER in "hw/acpi/bios-linker-loader.c", 
for the same. (I rewrote the types and the comments in edk2 from scratch, both 
for coding style reasons and for clearer documentation.)

... To be clear: the top-level pointers must exist somewhere (in some blob), 
because that helps edk2 find the tables (in some other blobs). However, the 
top-level pointers themselves don't need to reside in any ACPI table (RSDT, 
XSDT); they can just live in an otherwise unreferenced portion of one of the 
blobs.

But, IMO, implementing that wouldn't be much easier (and it would certainly be 
uglier) than composing a correct RSDT or XSDT. The latter would also keep the 
similarity with the x86 SeaBIOS case (where the RSDT is a hard requirement).

Thanks
Laszlo



Re: [Qemu-devel] [PATCH v4 14/20] hw/acpi/aml-build: Add aml_or() term

2015-04-09 Thread Igor Mammedov
On Fri, 3 Apr 2015 18:03:46 +0800
Shannon Zhao  wrote:

> From: Shannon Zhao 
> 
> Add aml_or() term and make aml_and can take three args.
> Expose build_append_int_noprefix as it wiil be used by
> creating a buffer.
> 
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/acpi/aml-build.c | 24 +---
>  hw/i386/acpi-build.c|  2 +-
>  include/hw/acpi/aml-build.h |  4 +++-
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 5a94fc9..312afb6 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -240,7 +240,7 @@ static void build_extop_package(GArray *package, uint8_t 
> op)
>  build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
>  }
>  
> -static void build_append_int_noprefix(GArray *table, uint64_t value, int 
> size)
> +void build_append_int_noprefix(GArray *table, uint64_t value, int size)
>  {
>  int i;
>  
> @@ -445,12 +445,30 @@ Aml *aml_store(Aml *val, Aml *target)
>  }
>  
>  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> -Aml *aml_and(Aml *arg1, Aml *arg2)
> +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
I know that it's possible to Store inside of And(a, b, save_here) ASL op,
but could you instead rewrite it to

 Store(And(a, b), save_here)

so it wouldn't clatter trivial  And(a,b) uses and drop this hunk.

>  {
>  Aml *var = aml_opcode(0x7B /* AndOp */);
>  aml_append(var, arg1);
>  aml_append(var, arg2);
> -build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +if (arg3 == NULL) {
> +build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +} else {
> +aml_append(var, arg3);
> +}
> +return var;
> +}
> +
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefOr */
> +Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3)
same here for arg3

> +{
> +Aml *var = aml_opcode(0x7D /* OrOp */);
> +aml_append(var, arg1);
> +aml_append(var, arg2);
> +if (arg3 == NULL) {
> +build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +} else {
> +aml_append(var, arg3);
> +}
>  return var;
>  }
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 7b5210e..133685e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -452,7 +452,7 @@ static void build_append_pcihp_notify_entry(Aml *method, 
> int slot)
>  Aml *if_ctx;
>  int32_t devfn = PCI_DEVFN(slot, 0);
>  
> -if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot)));
> +if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL));
>  aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1)));
>  aml_append(method, if_ctx);
>  }
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 942d986..3473d6e 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -156,7 +156,8 @@ Aml *aml_return(Aml *val);
>  Aml *aml_int(const uint64_t val);
>  Aml *aml_arg(int pos);
>  Aml *aml_store(Aml *val, Aml *target);
> -Aml *aml_and(Aml *arg1, Aml *arg2);
> +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3);
> +Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3);
>  Aml *aml_notify(Aml *arg1, Aml *arg2);
>  Aml *aml_call1(const char *method, Aml *arg1);
>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
> @@ -211,6 +212,7 @@ Aml *aml_field(const char *name, AmlFieldFlags flags);
>  Aml *aml_varpackage(uint32_t num_elements);
>  Aml *aml_touuid(int32_t val1, int16_t val2, int16_t val3,
>  int16_t val4, int64_t val5);
> +void build_append_int_noprefix(GArray *table, uint64_t value, int size);
>  
>  void
>  build_header(GArray *linker, GArray *table_data,




[Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

2015-04-09 Thread Eduardo Habkost
This helps us avoid memory leaks when using object_property_add_alias(), as it
is not practical for callers to save target_name to free it later.

Eduardo Habkost (2):
  qom: strdup() target property name on object_property_add_alias()
  qdev: Free property names after registering gpio aliases

 hw/core/qdev.c | 2 ++
 qom/object.c   | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.1.0




Re: [Qemu-devel] [Qemu-block] [PATCH] block/iscsi: handle zero events from iscsi_which_events

2015-04-09 Thread Stefan Hajnoczi
On Tue, Apr 7, 2015 at 9:08 PM, Peter Lieven  wrote:

CCing the entirety of this patch to qemu-devel@nongnu.org.  It is also
available in the qemu-bl...@nongnu.org mailing list archives:
http://permalink.gmane.org/gmane.comp.emulators.qemu.block/460

> newer libiscsi versions may return zero events from iscsi_which_events.
>
> In this case iscsi_service will return immediately without any progress.
> To avoid busy waiting for iscsi_which_events to change we deregister all
> read and write handlers in this case and schedule a timer to periodically
> check iscsi_which_events for changed events.
>
> Next libiscsi version will introduce async reconnects and zero events
> are returned while libiscsi is waiting for a reconnect retry.
>
> Signed-off-by: Peter Lieven 
> ---
>  block/iscsi.c |   33 +++--
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 3e34b1f..ba33290 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -56,6 +56,7 @@ typedef struct IscsiLun {
>  uint64_t num_blocks;
>  int events;
>  QEMUTimer *nop_timer;
> +QEMUTimer *event_timer;
>  uint8_t lbpme;
>  uint8_t lbprz;
>  uint8_t has_write_same;
> @@ -95,6 +96,7 @@ typedef struct IscsiAIOCB {
>  #endif
>  } IscsiAIOCB;
>
> +#define EVENT_INTERVAL 250
>  #define NOP_INTERVAL 5000
>  #define MAX_NOP_FAILURES 3
>  #define ISCSI_CMD_RETRIES ARRAY_SIZE(iscsi_retry_times)
> @@ -256,21 +258,30 @@ static void
>  iscsi_set_events(IscsiLun *iscsilun)
>  {
>  struct iscsi_context *iscsi = iscsilun->iscsi;
> -int ev;
> +int ev = iscsi_which_events(iscsi);
>
> -/* We always register a read handler.  */
> -ev = POLLIN;
> -ev |= iscsi_which_events(iscsi);
>  if (ev != iscsilun->events) {
>  aio_set_fd_handler(iscsilun->aio_context,
> iscsi_get_fd(iscsi),
> -   iscsi_process_read,
> +   (ev & POLLIN) ? iscsi_process_read : NULL,
> (ev & POLLOUT) ? iscsi_process_write : NULL,
> iscsilun);
> +iscsilun->events = ev;
> +}
>
> +/* newer versions of libiscsi may return zero events. In this
> + * case start a timer to ensure we are able to return to service
> + * once this situation changes. */
> +if (!ev) {
> +timer_mod(iscsilun->event_timer,
> +  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + EVENT_INTERVAL);
>  }
> +}
>
> -iscsilun->events = ev;
> +static void iscsi_timed_set_events(void *opaque)
> +{
> +IscsiLun *iscsilun = opaque;
> +iscsi_set_events(iscsilun);
>  }
>
>  static void
> @@ -1214,6 +1225,11 @@ static void iscsi_detach_aio_context(BlockDriverState 
> *bs)
>  timer_free(iscsilun->nop_timer);
>  iscsilun->nop_timer = NULL;
>  }
> +if (iscsilun->event_timer) {
> +timer_del(iscsilun->event_timer);
> +timer_free(iscsilun->event_timer);
> +iscsilun->event_timer = NULL;
> +}
>  }
>
>  static void iscsi_attach_aio_context(BlockDriverState *bs,
> @@ -1230,6 +1246,11 @@ static void iscsi_attach_aio_context(BlockDriverState 
> *bs,
>  iscsi_nop_timed_event, iscsilun);
>  timer_mod(iscsilun->nop_timer,
>qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
> +
> +/* Prepare a timer for a delayed call to iscsi_set_events */
> +iscsilun->event_timer = aio_timer_new(iscsilun->aio_context,
> +  QEMU_CLOCK_REALTIME, SCALE_MS,
> +  iscsi_timed_set_events, iscsilun);
>  }
>
>  static bool iscsi_is_write_protected(IscsiLun *iscsilun)
> --
> 1.7.9.5
>
>



Re: [Qemu-devel] [PATCH] translate-all: use g_malloc0 for all page descriptor allocations

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 19:24, Emilio G. Cota wrote:
> Since commit
> 
>   b7b5233a "bsd-user/mmap.c: Don't try to override g_malloc/g_free"
> 
> the exception we make here for usermode has been unnecessary.
> Get rid of it.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  translate-all.c | 18 ++
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/translate-all.c b/translate-all.c
> index 11763c6..3a6d116 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -389,18 +389,6 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  void **lp;
>  int i;
>  
> -#if defined(CONFIG_USER_ONLY)
> -/* We can't use g_malloc because it may recurse into a locked mutex. */
> -# define ALLOC(P, SIZE) \
> -do {\
> -P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,\
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
> -} while (0)
> -#else
> -# define ALLOC(P, SIZE) \
> -do { P = g_malloc0(SIZE); } while (0)
> -#endif
> -
>  /* Level 1.  Always allocated.  */
>  lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
>  
> @@ -412,7 +400,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  if (!alloc) {
>  return NULL;
>  }
> -ALLOC(p, sizeof(void *) * V_L2_SIZE);
> +p = g_malloc0(sizeof(void *) * V_L2_SIZE);

Using g_new0 would be even better. :)

Paolo

>  *lp = p;
>  }
>  
> @@ -424,12 +412,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  if (!alloc) {
>  return NULL;
>  }
> -ALLOC(pd, sizeof(PageDesc) * V_L2_SIZE);
> +pd = g_malloc0(sizeof(PageDesc) * V_L2_SIZE);
>  *lp = pd;
>  }
>  
> -#undef ALLOC
> -
>  return pd + (index & (V_L2_SIZE - 1));
>  }
>  
> 



Re: [Qemu-devel] [PATCH 14/14] target-arm: Check watchpoints against CPU security state

2015-04-09 Thread Edgar E. Iglesias
On Tue, Apr 07, 2015 at 09:10:00PM +0100, Peter Maydell wrote:
> Fix a TODO in bp_wp_matches() now that we have a function for
> testing whether the CPU is currently in Secure mode or not.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  target-arm/op_helper.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index ce09ab3..d8a1054 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -600,8 +600,10 @@ static bool bp_wp_matches(ARMCPU *cpu, int n, bool is_wp)
>  CPUARMState *env = &cpu->env;
>  uint64_t cr;
>  int pac, hmc, ssc, wt, lbn;
> -/* TODO: check against CPU security state when we implement TrustZone */
> -bool is_secure = false;
> +/* Note that for watchpoints the check is against the CPU security
> + * state, not the S/NS attribute on the offending data access.
> + */
> +bool is_secure = arm_is_secure(env);
>  int access_el = arm_current_el(env);
>  
>  if (is_wp) {
> -- 
> 1.9.1
> 



Re: [Qemu-devel] [PATCH] qemu-config: Accept empty option values

2015-04-09 Thread Eduardo Habkost
On Thu, Apr 09, 2015 at 02:11:11PM +0200, Paolo Bonzini wrote:
> On 08/04/2015 20:16, Eduardo Habkost wrote:
> > Currently it is impossible to set an option in a config file to an empty
> > string, because the parser matches only lines containing non-empty
> > strings between double-quotes.
> > 
> > As sscanf() "[" conversion specifier only matches non-empty strings, add
> > a special case for empty strings.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  util/qemu-config.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/util/qemu-config.c b/util/qemu-config.c
> > index 2d32ce7..9f9577d 100644
> > --- a/util/qemu-config.c
> > +++ b/util/qemu-config.c
> > @@ -413,7 +413,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, 
> > const char *fname)
> >  opts = qemu_opts_create(list, NULL, 0, &error_abort);
> >  continue;
> >  }
> > -if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
> > +if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
> > +(value[0] = '\0', sscanf(line, " %63s = \"\"", arg) == 1)) {
> >  /* arg = value */
> >  if (opts == NULL) {
> >  error_report("no group defined");
> > 
> 
> Acked-by: Paolo Bonzini 

Thanks! Whose tree this should go through?

-- 
Eduardo



Re: [Qemu-devel] [PATCH v4 04/20] hw/acpi/aml-build: Add aml_memory32_fixed() term

2015-04-09 Thread Igor Mammedov
On Fri, 3 Apr 2015 18:03:36 +0800
Shannon Zhao  wrote:

> From: Shannon Zhao 
> 
> Add aml_memory32_fixed() for describing device mmio region in resource 
> template.
> These can be used to generating DSDT table for ACPI on ARM.
> 
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/acpi/aml-build.c | 22 ++
>  include/hw/acpi/aml-build.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 8d01959..fefe7c7 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, 
> Aml *arg3, Aml *arg4)
>  return var;
>  }
>  
> +/*
> + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
> + */
> +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
perhaps
s/uint64_t/uint32_t/

don't use uintXX for bitfields if could be helped.
Probably in this case existing AmlReadAndWrite enum should be used.

> +{
> +Aml *var = aml_alloc();
> +build_append_byte(var->buf, 0x86); /* Memory32Fixed Resource Descriptor 
> */
> +build_append_byte(var->buf, 9); /* Length, bits[7:0] value = 9 */
> +build_append_byte(var->buf, 0); /* Length, bits[15:8] value = 0 */
> +build_append_byte(var->buf, rw_flag); /* Write status, 1 rw 0 ro */
> +build_append_byte(var->buf, addr & 0xff); /* Range base address 
> bits[7:0] */
> +build_append_byte(var->buf, (addr >> 8) & 0xff); /* Range base address 
> bits[15:8] */
> +build_append_byte(var->buf, (addr >> 16) & 0xff); /* Range base address 
> bits[23:16] */
> +build_append_byte(var->buf, (addr >> 24) & 0xff); /* Range base address 
> bits[31:24] */
> +
> +build_append_byte(var->buf, size & 0xff); /* Range length bits[7:0] */
> +build_append_byte(var->buf, (size >> 8) & 0xff); /* Range length 
> bits[15:8] */
> +build_append_byte(var->buf, (size >> 16) & 0xff); /* Range length 
> bits[23:16] */
> +build_append_byte(var->buf, (size >> 24) & 0xff); /* Range length 
> bits[31:24] */
> +return var;
> +}
> +
>  /* ACPI 1.0b: 6.4.2.5 I/O Port Descriptor */
>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>  uint8_t aln, uint8_t len)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 1705001..baa0652 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -162,6 +162,7 @@ Aml *aml_call1(const char *method, Aml *arg1);
>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml 
> *arg4);
> +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag);
>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>  uint8_t aln, uint8_t len);
>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,




Re: [Qemu-devel] [PATCH v4 05/20] hw/acpi/aml-build: Add aml_interrupt() term

2015-04-09 Thread Igor Mammedov
On Thu, 9 Apr 2015 14:09:23 +0800
Shannon Zhao  wrote:

> On 2015/4/8 22:57, Alex Bennée wrote:
> > 
> > Shannon Zhao  writes:
> > 
> >> From: Shannon Zhao 
> >>
> >> Add aml_interrupt() for describing device interrupt in resource template.
> >> These can be used to generating DSDT table for ACPI on ARM.
> >>
> >> Signed-off-by: Shannon Zhao 
> >> Signed-off-by: Shannon Zhao 
> >> ---
> >>  hw/acpi/aml-build.c | 18 ++
> >>  include/hw/acpi/aml-build.h |  1 +
> >>  2 files changed, 19 insertions(+)
> >>
> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >> index fefe7c7..bd1713c 100644
> >> --- a/hw/acpi/aml-build.c
> >> +++ b/hw/acpi/aml-build.c
> >> @@ -527,6 +527,24 @@ Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, 
> >> uint8_t rw_flag)
> >>  return var;
> >>  }
> >>  
> >> +/*
> >> + * ACPI 1.0: 6.4.3.6 Interrupt (Interrupt Resource Descriptor Macro)
> >> + */
> >> +Aml *aml_interrupt(uint8_t irq_flags, int irq)
> >> +{
> >> +Aml *var = aml_alloc();
> >> +build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
> >> +build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 
> >> 6 */
> >> +build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 
> >> 0 */
> >> +build_append_byte(var->buf, irq_flags); /* Interrupt Vector
> >> Information. */
> > 
> > As the spec says [7:4] is RES0 we might want to assert this is the case.
> > 
> 
> Yes, we should check although the probability is very small.
It's revision specific and we don't have infrastructure to check/validate
per revision differences.

I'd  split irq_flags from bitmask to a several args, a enum for each implemented
bit to avoid user setting reserved bits.

/* ACPI X.X: ... */
AmlWakeCap {
aml_not_wake_capable = 0,
aml_wake_capable = 1
}

...

> But the reserve bits are different in ACPI 5.1.
> 
> Bit[7:5] Reserved (must be 0)
> Bit[4] Wake Capability, _WKC
> 
> >> +build_append_byte(var->buf, 0x01); /* Interrupt table length = 1 */
> >> +build_append_byte(var->buf, irq & 0xff); /* Interrupt Number 
> >> bits[7:0] */
> >> +build_append_byte(var->buf, (irq >> 8) & 0xff); /* Interrupt Number 
> >> bits[15:8] */
> >> +build_append_byte(var->buf, (irq >> 16) & 0xff); /* Interrupt Number 
> >> bits[23:16] */
> >> +build_append_byte(var->buf, (irq >> 24) & 0xff); /* Interrupt
> >> Number bits[31:24] */
> > 
> > Again extractNN bitops?
> > 
> >> +return var;
> >> +}
> >> +
> >>  /* ACPI 1.0b: 6.4.2.5 I/O Port Descriptor */
> >>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> >>  uint8_t aln, uint8_t len)
> >> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> >> index baa0652..315c729 100644
> >> --- a/include/hw/acpi/aml-build.h
> >> +++ b/include/hw/acpi/aml-build.h
> >> @@ -163,6 +163,7 @@ Aml *aml_call2(const char *method, Aml *arg1, Aml 
> >> *arg2);
> >>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
> >>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml 
> >> *arg4);
> >>  Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag);
> >> +Aml *aml_interrupt(uint8_t irq_flags, int irq);
> >>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> >>  uint8_t aln, uint8_t len);
> >>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > 
> 




Re: [Qemu-devel] [PATCH for-2.3] configure: disable by default and warn about libxseg GPLv3 license

2015-04-09 Thread Andreas Färber
Am 09.04.2015 um 15:52 schrieb Stefan Hajnoczi:
> libxseg has changed license to GPLv3.  QEMU includes GPL "v2 only" code
> which is not compatible with GPLv3.  This means the resulting binaries
> may not be redistributable!
> 
> Disable Archipelago (libxseg) by default to prevent accidental license
> violations.  Also warn if linking against libxseg is enabled to remind
> the user.
> 
> Note that this commit does not constitute any advice about software
> licensing.  If you have doubts you should consult a lawyer.

:)

> 
> Cc: Chrysostomos Nanakos 
> Suggested-by: Kevin Wolf 
> Reported-by: Andreas Färber 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  configure | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> While libxseg reconsiders its license, let's disable it by default and warn
> users that they need to carefully check whether the licenses are compatible.
> 
> diff --git a/configure b/configure
> index 09c9225..9219ba3 100755
> --- a/configure
> +++ b/configure
> @@ -327,7 +327,7 @@ seccomp=""
>  glusterfs=""
>  glusterfs_discard="no"
>  glusterfs_zerofill="no"
> -archipelago=""
> +archipelago="no"
>  gtk=""
>  gtkabi=""
>  vte=""
> @@ -3173,6 +3173,12 @@ EOF
>  archipelago="yes"
>  libs_tools="$archipelago_libs $libs_tools"
>  libs_softmmu="$archipelago_libs $libs_softmmu"
> +
> + echo "WARNING: Please check the licenses of QEMU and libxseg carefully."
> + echo "GPLv3 versions of libxseg may not be compatible with QEMU's "

FWIW trailing space in argument (in case you send a pull yourself)

> + echo "license and therefore prevent redistribution."
> + echo
> + echo "To disable Archipelago, use --disable-archipelago"

Since you change the default, that's not strictly necessary any more but
not wrong either.

>  else
>if test "$archipelago" = "yes" ; then
>  feature_not_found "Archipelago backend support" "Install libxseg 
> devel"

Reviewed-by: Andreas Färber 

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



Re: [Qemu-devel] [RFC PATCH 0/3] pflash_cfi01: allow reading/writing it only in secure mode

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 14:06, Paolo Bonzini  wrote:
> On 09/04/2015 14:47, Peter Maydell wrote:
>> Are real flash devices ever wired up like this?
>
> On x86 machines it is almost exactly like this.  I'm implementing x86
> system management mode, and I'm reusing MEMTXATTRS_SECURE for it.

Cool -- useful to have a non-ARM use of some of this, helps
to keep the design of the common-code parts honest.

-- PMM



Re: [Qemu-devel] [PATCH 01/12 v9] linux-user: tilegx: Firstly add architecture related features

2015-04-09 Thread Peter Maydell
On 27 March 2015 at 10:48, Chen Gang  wrote:
> They are based on Linux kernel tilegx architecture for 64 bit binary,
> also based on tilegx ABI reference document.
>
> Signed-off-by: Chen Gang 
> ---
>  linux-user/tilegx/syscall.h|  80 
>  linux-user/tilegx/syscall_nr.h | 278 
>  linux-user/tilegx/termbits.h   | 285 
> +
>  3 files changed, 643 insertions(+)
>  create mode 100644 linux-user/tilegx/syscall.h
>  create mode 100644 linux-user/tilegx/syscall_nr.h
>  create mode 100644 linux-user/tilegx/termbits.h
>
> diff --git a/linux-user/tilegx/syscall.h b/linux-user/tilegx/syscall.h
> new file mode 100644
> index 000..561e158
> --- /dev/null
> +++ b/linux-user/tilegx/syscall.h
> @@ -0,0 +1,80 @@
> +#ifndef TILEGX_SYSCALLS_H
> +#define TILEGX_SYSCALLS_H
> +
> +#define UNAME_MACHINE "tilegx"
> +#define UNAME_MINIMUM_RELEASE "3.19"
> +
> +/* We use tilegx to keep things similar to the kernel sources.  */

This is true but a slightly odd place to say so.

> +typedef uint64_t tilegx_reg_t;
> +
> +struct target_pt_regs {
> +
> +/* Can be as parameters */
> +tilegx_reg_t r0;  /* Also for return value, both function and system 
> call */
> +tilegx_reg_t r1;
> +tilegx_reg_t r2;
> +tilegx_reg_t r3;
> +tilegx_reg_t r4;
> +tilegx_reg_t r5;
> +tilegx_reg_t r6;
> +tilegx_reg_t r7;
> +tilegx_reg_t r8;
> +tilegx_reg_t r9;
> +
> +/* Normal using, caller saved */
> +tilegx_reg_t r10;  /* Also for system call */
> +tilegx_reg_t r11;
> +tilegx_reg_t r12;
> +tilegx_reg_t r13;
> +tilegx_reg_t r14;
> +tilegx_reg_t r15;
> +tilegx_reg_t r16;
> +tilegx_reg_t r17;
> +tilegx_reg_t r18;
> +tilegx_reg_t r19;
> +tilegx_reg_t r20;
> +tilegx_reg_t r21;
> +tilegx_reg_t r22;
> +tilegx_reg_t r23;
> +tilegx_reg_t r24;
> +tilegx_reg_t r25;
> +tilegx_reg_t r26;
> +tilegx_reg_t r27;
> +tilegx_reg_t r28;
> +tilegx_reg_t r29;
> +
> +/* Normal using, callee saved */
> +tilegx_reg_t r30;
> +tilegx_reg_t r31;
> +tilegx_reg_t r32;
> +tilegx_reg_t r33;
> +tilegx_reg_t r34;
> +tilegx_reg_t r35;
> +tilegx_reg_t r36;
> +tilegx_reg_t r37;
> +tilegx_reg_t r38;
> +tilegx_reg_t r39;
> +tilegx_reg_t r40;
> +tilegx_reg_t r41;
> +tilegx_reg_t r42;
> +tilegx_reg_t r43;
> +tilegx_reg_t r44;
> +tilegx_reg_t r45;
> +tilegx_reg_t r46;
> +tilegx_reg_t r47;
> +tilegx_reg_t r48;
> +tilegx_reg_t r49;
> +tilegx_reg_t r50;
> +tilegx_reg_t r51;
> +
> +/* Control using */
> +tilegx_reg_t r52;/* optional frame pointer */

Why aren't we using an array, the way the kernel does?

> +tilegx_reg_t tp; /* thread-local data */
> +tilegx_reg_t sp; /* stack pointer */
> +tilegx_reg_t lr; /* lr pointer */

This is missing a bunch of stuff from the kernel uapi
pt_regs type, which is bad because this struct is part
of the user-facing ABI (it gets used in signal handling).

> diff --git a/linux-user/tilegx/termbits.h b/linux-user/tilegx/termbits.h
> new file mode 100644
> index 000..c11ce3e
> --- /dev/null
> +++ b/linux-user/tilegx/termbits.h

> +#define TARGET_TIOCNOTTY0x5422
> +#define TARGET_TIOCSETD 0x5423
> +#define TARGET_TIOCGETD 0x5424
> +#define TARGET_TCSBRKP  0x5425
> +#define TARGET_TIOCSBRK 0x5427
> +#define TARGET_TIOCCBRK 0x5428
> +#define TARGET_TIOCGSID 0x5429
> +#define TARGET_TCGETS2  _IOR('T', 0x2A, struct termios2)

You probably mean TARGET_IOR/TARGET_IOW here and below.


> +#define TARGET_TCSETS2  _IOW('T', 0x2B, struct termios2)
> +#define TARGET_TCSETSW2 _IOW('T', 0x2C, struct termios2)
> +#define TARGET_TCSETSF2 _IOW('T', 0x2D, struct termios2)
> +#define TARGET_TIOCGRS485   0x542E
> +#define TARGET_TIOCSRS485   0x542F
> +#define TARGET_TIOCGPTN _IOR('T', 0x30, unsigned int)
> +#define TARGET_TIOCSPTLCK   _IOW('T', 0x31, int)
> +#define TARGET_TIOCGDEV _IOR('T', 0x32, unsigned int)
> +#define TARGET_TCGETX   0x5432
> +#define TARGET_TCSETX   0x5433
> +#define TARGET_TCSETXF  0x5434
> +#define TARGET_TCSETXW  0x5435
> +#define TARGET_TIOCSIG  _IOW('T', 0x36, int)
> +#define TARGET_TIOCVHANGUP  0x5437
> +#define TARGET_TIOCGPKT _IOR('T', 0x38, int)
> +#define TARGET_TIOCGPTLCK   _IOR('T', 0x39, int)
> +#define TARGET_TIOCGEXCL_IOR('T', 0x40, int)

thanks
-- PMM



Re: [Qemu-devel] [PULL 22/62] block: Support Archipelago as a QEMU block backend

2015-04-09 Thread Stefan Hajnoczi
On Thu, Apr 9, 2015 at 1:48 PM, Chrysostomos Nanakos  wrote:
> On 2015-04-09 06:48, Andreas Färber wrote:
>>
>> Am 08.08.2014 um 19:39 schrieb Kevin Wolf:
>>>
>>> From: Chrysostomos Nanakos 
>>>
>>> VM Image on Archipelago volume is specified like this:
>>>
>>>
>>>
>>> file.driver=archipelago,file.volume=[,file.mport=[,
>>> file.vport=][,file.segment=]]
>>>
>>> 'archipelago' is the protocol.
>>>
>>> 'mport' is the port number on which mapperd is listening. This is
>>> optional
>>> and if not specified, QEMU will make Archipelago to use the default port.
>>>
>>> 'vport' is the port number on which vlmcd is listening. This is optional
>>> and if not specified, QEMU will make Archipelago to use the default port.
>>>
>>> 'segment' is the name of the shared memory segment Archipelago stack is
>>> using.
>>> This is optional and if not specified, QEMU will make Archipelago to use
>>> the
>>> default value, 'archipelago'.
>>>
>>> Examples:
>>>
>>> file.driver=archipelago,file.volume=my_vm_volume
>>> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123
>>> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
>>> file.vport=1234
>>> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
>>> file.vport=1234,file.segment=my_segment
>>>
>>> Signed-off-by: Chrysostomos Nanakos 
>>> Reviewed-by: Stefan Hajnoczi 
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  MAINTAINERS |   6 +
>>>  block/Makefile.objs |   2 +
>>>  block/archipelago.c | 787
>>> 
>>>  configure   |  40 +++
>>>  4 files changed, 835 insertions(+)
>>>  create mode 100644 block/archipelago.c
>>
>>
>> Judging by configure output in v2.3.0-rc2, QEMU seems to rely on
>> libxseg, which is GPL-3.0+: https://github.com/grnet/libxseg
>>
>> How can anyone legally build this backend then? o.O
>>
>> Any chance libxseg can be relicensed to GPL-2.0+?
>>
>
> Hello Andreas,
> indeed the license has changed a few months after submitting the patch for
> the block driver to QEMU. I have already contacted the administration
> of GRNET which is responsible for this change asking them about the
> aforementioned problem. I am waiting for the reply and hopefully we will
> resolve this license matter.

In the meantime I've sent a patch to disable Archipelago by default in
./configure ("[PATCH for-2.3] configure: disable by default and warn
about libxseg GPLv3 license").  You are CCed on the email.

The intention is to reduce the chance of users accidentally compiling
binaries with license problems.  Archipelago support is still
available but it must be explicitly enabled using ./configure
--enable-archipelago.

Stefan



Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-04-09 Thread Alberto Garcia
On Wed, Apr 08, 2015 at 10:27:34AM -0600, Eric Blake wrote:

> > +++ b/block/snapshot.c
> > @@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> >  if (bs->file) {
> >  return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
> >  }
> > -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > -  drv->format_name, bdrv_get_device_name(bs),
> > -  "internal snapshot deletion");
> > +error_setg(errp, "Block format '%s' used by device '%s' "
> > +   "does not support internal snapshot deletion",
> > +   drv->format_name, bdrv_get_device_name(bs));
> >  return -ENOTSUP;
> >  }
> >  
> > @@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
> >  if (drv->bdrv_snapshot_load_tmp) {
> >  return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
> >  }
> > -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > -  drv->format_name, bdrv_get_device_name(bs),
> > -  "temporarily load internal snapshot");
> > +error_setg(errp, "Block format '%s' used by device '%s' "
> > +   "does not support temporarily loading internal snapshots",
> > +   drv->format_name, bdrv_get_device_name(bs));
> 
> Should these two messages use 'node' instead of 'device'?  After
> all, a format is tied to a node (as a backing chain can involve
> multiple nodes using different formats)
> 
> > +++ b/blockdev.c
> >  if (!bdrv_can_snapshot(bs)) {
> > -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > -  bs->drv->format_name, device, "internal snapshot");
> > +error_setg(errp, "Block format '%s' used by device '%s' "
> > +   "does not support internal snapshots",
> > +   bs->drv->format_name, device);
> 
> but this is probably another one where node may be better.

I decided to leave 'device' in all cases where 'bs' cannot possibly be
anything else that a root node.

In this latter case, for example, that bs is obtained using
blk_bs(blk_by_name(device)).

Berto



Re: [Qemu-devel] [PATCH 0/3] Add support for for GICv2m and MSIs to arm-virt

2015-04-09 Thread Christoffer Dall
On Thu, Apr 9, 2015 at 12:01 AM, Nikolay Nikolaev
 wrote:
> On Thu, Apr 9, 2015 at 12:20 AM, Christoffer Dall
>  wrote:
>> Now when we have a host generic PCIe controller in the virt board, it
>> would be nice to be able to use MSIs so that we can eventually enable
>> VHOST with KVM.
>>
>> With these patches you can use MSIs with TCG and with KVM, but you still
>> need some fixes for the mapping of the IRQ index to the GSI number for
>> IRQFD to work.  A separate patch series will follow this one to enable
>> that.
>>
>> Tested with KVM on XGene and with TCG by configuring a virtio-pci
>> network adapter for the guest and verifying MSIs going through as
>> expected.
>
> Christofer, have you measured the network performance difference with KVM.
> Probably the next patchseries (with IRQFD) makes bigger difference.
>
I did run numbers, and it depends greatly on the workload, but for
something simple like netperf, you see a huge increase in performance
(several orders of magnitude).  Surprisingly, a large portion of that
seems to come from using ioeventfd for the mmio accesses to vhost,
possibly because it begins to use multiple queues.  For latency
measurements, irqfd also makes a big difference.

-Christoffer



[Qemu-devel] [PULL 4/4] block/iscsi: handle zero events from iscsi_which_events

2015-04-09 Thread Stefan Hajnoczi
From: Peter Lieven 

newer libiscsi versions may return zero events from iscsi_which_events.

In this case iscsi_service will return immediately without any progress.
To avoid busy waiting for iscsi_which_events to change we deregister all
read and write handlers in this case and schedule a timer to periodically
check iscsi_which_events for changed events.

Next libiscsi version will introduce async reconnects and zero events
are returned while libiscsi is waiting for a reconnect retry.

Signed-off-by: Peter Lieven 
Message-id: 1428437295-29577-1-git-send-email...@kamp.de
Signed-off-by: Stefan Hajnoczi 
---
 block/iscsi.c | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3e34b1f..ba33290 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -56,6 +56,7 @@ typedef struct IscsiLun {
 uint64_t num_blocks;
 int events;
 QEMUTimer *nop_timer;
+QEMUTimer *event_timer;
 uint8_t lbpme;
 uint8_t lbprz;
 uint8_t has_write_same;
@@ -95,6 +96,7 @@ typedef struct IscsiAIOCB {
 #endif
 } IscsiAIOCB;
 
+#define EVENT_INTERVAL 250
 #define NOP_INTERVAL 5000
 #define MAX_NOP_FAILURES 3
 #define ISCSI_CMD_RETRIES ARRAY_SIZE(iscsi_retry_times)
@@ -256,21 +258,30 @@ static void
 iscsi_set_events(IscsiLun *iscsilun)
 {
 struct iscsi_context *iscsi = iscsilun->iscsi;
-int ev;
+int ev = iscsi_which_events(iscsi);
 
-/* We always register a read handler.  */
-ev = POLLIN;
-ev |= iscsi_which_events(iscsi);
 if (ev != iscsilun->events) {
 aio_set_fd_handler(iscsilun->aio_context,
iscsi_get_fd(iscsi),
-   iscsi_process_read,
+   (ev & POLLIN) ? iscsi_process_read : NULL,
(ev & POLLOUT) ? iscsi_process_write : NULL,
iscsilun);
+iscsilun->events = ev;
+}
 
+/* newer versions of libiscsi may return zero events. In this
+ * case start a timer to ensure we are able to return to service
+ * once this situation changes. */
+if (!ev) {
+timer_mod(iscsilun->event_timer,
+  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + EVENT_INTERVAL);
 }
+}
 
-iscsilun->events = ev;
+static void iscsi_timed_set_events(void *opaque)
+{
+IscsiLun *iscsilun = opaque;
+iscsi_set_events(iscsilun);
 }
 
 static void
@@ -1214,6 +1225,11 @@ static void iscsi_detach_aio_context(BlockDriverState 
*bs)
 timer_free(iscsilun->nop_timer);
 iscsilun->nop_timer = NULL;
 }
+if (iscsilun->event_timer) {
+timer_del(iscsilun->event_timer);
+timer_free(iscsilun->event_timer);
+iscsilun->event_timer = NULL;
+}
 }
 
 static void iscsi_attach_aio_context(BlockDriverState *bs,
@@ -1230,6 +1246,11 @@ static void iscsi_attach_aio_context(BlockDriverState 
*bs,
 iscsi_nop_timed_event, iscsilun);
 timer_mod(iscsilun->nop_timer,
   qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
+
+/* Prepare a timer for a delayed call to iscsi_set_events */
+iscsilun->event_timer = aio_timer_new(iscsilun->aio_context,
+  QEMU_CLOCK_REALTIME, SCALE_MS,
+  iscsi_timed_set_events, iscsilun);
 }
 
 static bool iscsi_is_write_protected(IscsiLun *iscsilun)
-- 
2.1.0




[Qemu-devel] [PATCH] aio: strengthen memory barriers for bottom half scheduling

2015-04-09 Thread Paolo Bonzini
There are two problems with memory barriers in async.c.  The fix is
to use atomic_xchg in order to achieve sequential consistency between
the scheduling of a bottom half and the corresponding execution.

First, if bh->scheduled is already 1 in qemu_bh_schedule, QEMU does
not execute a memory barrier to order any writes needed by the callback
before the read of bh->scheduled.  If the other side sees req->state as
THREAD_ACTIVE, the callback is not invoked and you get deadlock.

Second, the memory barrier in aio_bh_poll is too weak.  Without this
patch, it is possible that bh->scheduled = 0 is not "published" until
after the callback has returned.  Another thread wants to schedule the
bottom half, but it sees bh->scheduled = 1 and does nothing.  This causes
a lost wakeup.  The memory barrier should have been changed to smp_mb()
in commit 924fe12 (aio: fix qemu_bh_schedule() bh->ctx race condition,
2014-06-03) together with qemu_bh_schedule()'s.  Guess who reviewed
that patch?

Both of these involve a store and a load, so they are reproducible
on x86_64 as well.  Paul Leveille however reported how to trigger the
problem within 15 minutes on x86_64 as well.  His (untested) recipe,
reproduced here for reference, is the following:

   1) Qcow2 (or 3) is critical – raw files alone seem to avoid the problem.

   2) Use “cache=directsync” rather than the default of
   “cache=none” to make it happen easier.

   3) Use a server with a write-back RAID controller to allow for rapid
   IO rates.

   4) Run a random-access load that (mostly) writes chunks to various
   files on the virtual block device.

  a. I use ‘diskload.exe c:25’, a Microsoft HCT load
 generator, on Windows VMs.

  b. Iometer can probably be configured to generate a similar load.

   5) Run multiple VMs in parallel, against the same storage device,
   to shake the failure out sooner.

   6) IvyBridge and Haswell processors for certain; not sure about others.

A similar patch survived over 12 hours of testing, where an unpatched
QEMU would fail within 15 minutes.

This bug is, most likely, also involved in the failures in the libguestfs
testsuite on AArch64 (reported by Laszlo Ersek and Richard Jones).  However,
the patch is not enough to fix that.

Thanks to Stefan Hajnoczi for suggesting closer examination of
qemu_bh_schedule, and to Paul for providing test input and a prototype
patch.

Cc: qemu-sta...@nongnu.org
Reported-by: Laszlo Ersek 
Reported-by: Paul Leveille 
Reported-by: John Snow 
Suggested-by: Paul Leveille 
Suggested-by: Stefan Hajnoczi 
Tested-by: Paul Leveille 
Signed-off-by: Paolo Bonzini 
---
 async.c | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/async.c b/async.c
index 2be88cc..2b51e87 100644
--- a/async.c
+++ b/async.c
@@ -72,12 +72,13 @@ int aio_bh_poll(AioContext *ctx)
 /* Make sure that fetching bh happens before accessing its members */
 smp_read_barrier_depends();
 next = bh->next;
-if (!bh->deleted && bh->scheduled) {
-bh->scheduled = 0;
-/* Paired with write barrier in bh schedule to ensure reading for
- * idle & callbacks coming after bh's scheduling.
- */
-smp_rmb();
+/* The atomic_xchg is paired with the one in qemu_bh_schedule.  The
+ * implicit memory barrier ensures that the callback sees all writes
+ * done by the scheduling thread.  It also ensures that the scheduling
+ * thread sees the zero before bh->cb has run, and thus will call
+ * aio_notify again if necessary.
+ */
+if (!bh->deleted && atomic_xchg(&bh->scheduled, 0)) {
 if (!bh->idle)
 ret = 1;
 bh->idle = 0;
@@ -108,33 +109,28 @@ int aio_bh_poll(AioContext *ctx)
 
 void qemu_bh_schedule_idle(QEMUBH *bh)
 {
-if (bh->scheduled)
-return;
 bh->idle = 1;
 /* Make sure that idle & any writes needed by the callback are done
  * before the locations are read in the aio_bh_poll.
  */
-smp_wmb();
-bh->scheduled = 1;
+atomic_mb_set(&bh->scheduled, 1);
 }
 
 void qemu_bh_schedule(QEMUBH *bh)
 {
 AioContext *ctx;
 
-if (bh->scheduled)
-return;
 ctx = bh->ctx;
 bh->idle = 0;
-/* Make sure that:
+/* The memory barrier implicit in atomic_xchg makes sure that:
  * 1. idle & any writes needed by the callback are done before the
  *locations are read in the aio_bh_poll.
  * 2. ctx is loaded before scheduled is set and the callback has a chance
  *to execute.
  */
-smp_mb();
-bh->scheduled = 1;
-aio_notify(ctx);
+if (atomic_xchg(&bh->scheduled, 1) == 0) {
+aio_notify(ctx);
+}
 }
 
 
-- 
2.3.4




Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

2015-04-09 Thread Gerd Hoffmann
  Hi,

> Would it make sense for me to re-send just 1/5
> ("fw_cfg: add documentation file") to actually provide the
> above-mentioned documentation file with 2.3, leaving only
> patches 2..5 (the actual code changes, subject to the freeze)
> for 2.4 resubmission ?

Yes, doc updates are fine even in freeze.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 07/14] exec.c: Add new address_space_ld*/st* functions

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 13:49, Peter Maydell wrote:
>> > I think we do not want to expose these at all (or at least, all users
>> > should really be CPUs and hence use *_phys functions).
>> >
>> > S390 is always big-endian, and watch_mem_read/write can use the same
>> > buffer trick as subpages (and in fact should probably use memattrs as 
>> > well).
>> >
>> > So, please at least add a comment that these functions are deprecated,
>> > and check if watch_mem_read/write should be handled like subpages.
> I looked at the subpages code, and it seems to me that it's the
> other way around -- the subpages code should use these new functions.
> At the moment the subpage handlers use address_space_read/write
> to pull the data into a buffer, and then use the ldl_p/stl_p functions
> to do "read data from target-CPU order buffer into host variable".
> It would be better for them to just directly be able to say "do
> a ld/st in target-CPU order into this host variable", which is
> the purpose of these new functions. Indirecting via a buffer seems
> like an ugly workaround for not having the direct operation.

Using them in subpage code is fine, but then the subpage code is in
exec.c and can use the _internal version directly (and pass
DEVICE_NATIVE_ENDIAN).  Still, usage of these outside exec.c is probably
suspicious.  It's at least worth pulling these in cpu-all.h; the whole
contents of cpu-common.h look like a sundry of functions that either are
deprecated or should be declared elsewhere.

Paolo



Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

2015-04-09 Thread Liviu Ionescu

> On 09 Apr 2015, at 08:12, Stefan Weil  wrote:
> 
> The first four are custom packages, and you can get them from 
> http://qemu.weilnetz.de/debian/.
> The last one is Wheezy specific and not needed.

ok, got them, installed them and got no errors, they are now listed by dpkg -l

> The gtk custom packages were made from gtk.org downloads (all-in-one-bundles, 
> for example
> from http://www.gtk.org/download/win64.php). That is the same source which 
> Peter suggested.
> 
> The libfdt custom packages simple packed the results from a QEMU build with 
> internal libfdt:
> QEMU can be build without any external libfdt and will then compile it during 
> the normal build.
> As I run many builds and want to avoid this extra compilation step, I created 
> that packages.

ok, we'll return to the procedure you used to build them later, since I want to 
document it too.

> As you can see in the list above, all custom packages work for Wheezy and for 
> Jessie.

not yet. the packages were installed correctly, I can see the files in the 
corresponding /usr/x86_64-w64-mingw32 folders, but when you built these 
packages, you did not update the prefix in the pkg-config files, and most read 
something like "prefix=/srv/win32builder/fixed_364/build/win64", which might 
exist on your machine, but does not exist on a clean machine.

I guess you either tweaked the pkg-config to find them, or you set some 
environment variables before configure.

with the default settings, configure produces:

CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -mms-bitfields 
-I/srv/win32builder/fixed_364/build/win64/include/glib-2.0 
-I/srv/win32builder/fixed_364/build/win64/lib/glib-2.0/include  -g 
QEMU_CFLAGS   -I/srv/win32builder/fixed_364/build/win64/include/pixman-1   
-m64 -D__USE_MINGW_ANSI_STDIO=1 -DWIN32_LEAN_AND_MEAN -DWINVER=0x501 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -Wno-missing-format-attribute  -Wendif-labels 
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration 
-Wold-style-definition -Wtype-limits -fstack-protector-strong  
-I/srv/win32builder/fixed_364/build/win64/include/libpng15 
LDFLAGS   -Wl,--nxcompat -Wl,--no-seh -Wl,--dynamicbase 
-Wl,--warn-common -m64 -g 

which obviously breaks the make.

can you share these details too? I want to reproduce "exactly" your environment.


regards,

Liviu






[Qemu-devel] glusterfs-api.pc versioning breaks QEMU

2015-04-09 Thread Andreas Färber
Hello,

Testing QEMU v2.3.0-rc2, I have run into QEMU's glusterfs support being
broken on openSUSE Tumbleweed, resulting in its configure complaining:

[   76s] ERROR: User requested feature GlusterFS backend support
[   76s]configure was not able to find it.
[   76s]Install glusterfs-api devel >= 3

What our configure is doing is
 pkg-config --atleast-version=3 glusterfs-api
on success followed by
 pkg-config --atleast-version=5 glusterfs-api
and
 pkg-config --atleast-version=6 glusterfs-api

Here's a short overview of the glusterfs-api.pc versions:

release-3.4: "Version: 4"

v3.5.0..v3.5.3: "Version: 6"
v3.5.4beta1 and release-3.5: "Version: 4.${PACKAGE_VERSION}"

v3.6.0: "Version: 7.0.0"
v3.6.1: "Version: 0.0.0"
v3.6.2..v3.6.3beta2 and release-3.6: "Version: 4.${PACKAGE_VERSION}"

So, both 3.5 and 3.6 series have gone backwards in their version
compared to released versions and are thus not backwards-compatible,
unlike what the commit message claims.

openSUSE 13.1 and 13.2 and reportedly Fedora 21 are still at versions
with "Version: 6", so not yet affected. openSUSE Tumbleweed is still at
v3.6.1 (with 3 > 0.0.0), but even once I get it updated to v3.6.2 the
checks for versions 5 and 6 would still fail, disabling features.

Naively I would consider versions jumping backwards a bug in glusterfs,
so I wonder why you chose 4.* and not 6.* and 7.* respectively.

How do you expect this to be handled by packages such as QEMU?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] configure: improve multiarch support for pkgconfig

2015-04-09 Thread John Snow
This will improve the multi-arch compilation for hosts using gcc.
configurations using clang won't see an improvement, but also won't
see a regression.

Signed-off-by: John Snow 
---
 configure | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/configure b/configure
index 826d6fd..d27729a 100755
--- a/configure
+++ b/configure
@@ -1759,6 +1759,20 @@ if ! has "$pkg_config_exe"; then
 fi
 
 ##
+# pkg-config multi-arch probe
+
+if $cc -print-multiarch >/dev/null 2>&1; then
+mlibdir=$($cc -print-multiarch $QEMU_CFLAGS)
+fi
+if test -z "${mlibdir}"; then
+mlibdir=$($cc --print-multi-os-directory $QEMU_CFLAGS)
+fi
+
+if [ -n "${mlibdir}" ] && [ -d "/usr/lib/${mlibdir}/pkgconfig" ]; then
+export PKG_CONFIG_LIBDIR="/usr/lib/${mlibdir}/pkgconfig"
+fi
+
+##
 # NPTL probe
 
 if test "$linux_user" = "yes"; then
-- 
2.1.0




[Qemu-devel] [PATCH v3] target-i386: Register QOM properties for feature flags

2015-04-09 Thread Eduardo Habkost
This uses the feature name arrays to register "feat-*" QOM properties
for feature flags. This simply adds the properties so they can be
configured using -global, but doesn't change x86_cpu_parse_featurestr()
to use them yet.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use "cpuid-" prefix instead of "feat-"
* Register release function for property
* Convert '_' to '-' on feature name before registering property
* Add dev->realized check to property setter

Changes v2 -> v3:
* Register alias properties for feature name aliases
* Patch is based on x86 tree, at:
  https://github.com/ehabkost/qemu.git x86
---
 target-i386/cpu.c | 119 ++
 1 file changed, 119 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e657f10..b8ff89d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2848,12 +2848,124 @@ out:
 }
 }
 
+typedef struct FeatureProperty {
+FeatureWord word;
+uint32_t mask;
+} FeatureProperty;
+
+
+static void x86_cpu_get_feature_prop(Object *obj,
+ struct Visitor *v,
+ void *opaque,
+ const char *name,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+FeatureProperty *fp = opaque;
+bool value = (env->features[fp->word] & fp->mask) == fp->mask;
+visit_type_bool(v, &value, name, errp);
+}
+
+static void x86_cpu_set_feature_prop(Object *obj,
+ struct Visitor *v,
+ void *opaque,
+ const char *name,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+DeviceState *dev = DEVICE(obj);
+CPUX86State *env = &cpu->env;
+FeatureProperty *fp = opaque;
+bool value;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_bool(v, &value, name, errp);
+if (value) {
+env->features[fp->word] |= fp->mask;
+} else {
+env->features[fp->word] &= ~fp->mask;
+}
+}
+
+static void x86_cpu_release_feature_prop(Object *obj, const char *name,
+ void *opaque)
+{
+FeatureProperty *prop = opaque;
+g_free(prop);
+}
+
+/* Register a boolean feature-bits property.
+ * If mask has multiple bits, all must be set for the property to return true.
+ * The same property name can be registered multiple times to make it affect
+ * multiple bits in the same FeatureWord.
+ */
+static void x86_cpu_register_feature_prop(X86CPU *cpu,
+  const char *prop_name,
+  FeatureWord w,
+  uint32_t mask)
+{
+FeatureProperty *fp;
+ObjectProperty *op;
+op = object_property_find(OBJECT(cpu), prop_name, NULL);
+if (op) {
+fp = op->opaque;
+assert(fp->word == w);
+fp->mask |= mask;
+} else {
+fp = g_new0(FeatureProperty, 1);
+fp->word = w;
+fp->mask = mask;
+object_property_add(OBJECT(cpu), prop_name, "bool",
+x86_cpu_get_feature_prop,
+x86_cpu_set_feature_prop,
+x86_cpu_release_feature_prop, fp, &error_abort);
+}
+}
+
+static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
+   FeatureWord w,
+   int bit)
+{
+Object *obj = OBJECT(cpu);
+int i;
+char **names, *name0;
+FeatureWordInfo *fi = &feature_word_info[w];
+
+if (!fi->feat_names) {
+return;
+}
+if (!fi->feat_names[bit]) {
+return;
+}
+
+names = g_strsplit(fi->feat_names[bit], "|", 0);
+
+name0 = g_strdup_printf("cpuid-%s", names[0]);
+feat2prop(name0);
+x86_cpu_register_feature_prop(cpu, name0, w, (1UL << bit));
+
+for (i = 1; names[i]; i++) {
+char *prop_name = g_strdup_printf("cpuid-%s", names[i]);
+feat2prop(prop_name);
+object_property_add_alias(obj, prop_name, obj, name0, &error_abort);
+g_free(prop_name);
+}
+
+g_strfreev(names);
+}
+
 static void x86_cpu_initfn(Object *obj)
 {
 CPUState *cs = CPU(obj);
 X86CPU *cpu = X86_CPU(obj);
 X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
 CPUX86State *env = &cpu->env;
+FeatureWord w;
 static int inited;
 
 cs->env_ptr = env;
@@ -2894,6 +3006,13 @@ static void x86_cpu_initfn(Object *obj)
 cpu->apic_id = -1;
 #endif
 
+for (w = 0; w < FEATURE_WORDS; w++) {
+int bit;
+for (bit = 0; bit < 32; bit++) {
+x86_cpu_register_feature_bit_props(cpu, w, bit);
+}
+}
+
 x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
 
 /* init variou

Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 21:43, Stefan Weil  wrote:
> My fork also contains additional code which would need much work
> to integrate it in the official QEMU (MIPS AR7 with WLAN router emulation,
> Raspberry Pi emulation and other parts which are not always working).

As an aside, if you're interested in trying to get rpi support
upstream I'm happy to help -- we do seem to get a fair amount
of "how do I emulate rpi in QEMU" questions so it seems
worthwhile. If you don't want to (or if this is just somebody
else's project/patches you're carrying in your tree) I
understand that too -- I have a bunch of omap3 patches
out-of-tree I've never got round to upstreaming either...

-- PMM



Re: [Qemu-devel] [PATCH v4 08/20] hw/arm/virt-acpi-build: Generate MADT table

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> MADT describes GIC enabled ARM platforms. The GICC and GICD
> subtables are used to define the GIC regions.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Alex Bennée 

> ---
>  hw/arm/virt-acpi-build.c | 61 
> 
>  include/hw/acpi/acpi-defs.h  | 36 +++-
>  include/hw/arm/virt-acpi-build.h |  2 ++
>  3 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index cb8b030..c8245ef 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -57,6 +57,20 @@
>  #define ACPI_BUILD_DPRINTF(fmt, ...)
>  #endif
>  
> +typedef struct VirtAcpiCpuInfo {
> +DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
> +} VirtAcpiCpuInfo;
> +
> +static void virt_acpi_get_cpu_info(VirtAcpiCpuInfo *cpuinfo)
> +{
> +CPUState *cpu;
> +
> +memset(cpuinfo->found_cpus, 0, sizeof cpuinfo->found_cpus);
> +CPU_FOREACH(cpu) {
> +set_bit(cpu->cpu_index, cpuinfo->found_cpus);
> +}
> +}
> +
>  static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
>  {
>  Aml *dev, *crs;
> @@ -162,6 +176,47 @@ static void acpi_dsdt_add_virtio(Aml *scope, const 
> hwaddr *mmio_addrs,
>  }
>  }
>  
> +/* MADT */
> +static void
> +build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
> +   VirtAcpiCpuInfo *cpuinfo)
> +{
> +int madt_start = table_data->len;
> +const struct acpi_madt_info *info = guest_info->madt_info;
> +AcpiMultipleApicTable *madt;
> +AcpiMadtGenericDistributor *gicd;
> +int i;
> +
> +madt = acpi_data_push(table_data, sizeof *madt);
> +madt->local_apic_address = *info->gic_cpu_base_addr;
> +madt->flags = cpu_to_le32(1);
> +
> +for (i = 0; i < guest_info->max_cpus; i++) {
> +AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
> + sizeof *gicc);
> +gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
> +gicc->length = sizeof(*gicc);
> +gicc->base_address = *info->gic_cpu_base_addr;
> +gicc->cpu_interface_number = i;
> +gicc->arm_mpidr = i;
> +gicc->uid = i;
> +if (test_bit(i, cpuinfo->found_cpus)) {
> +gicc->flags = cpu_to_le32(1);
> +} else {
> +gicc->flags = cpu_to_le32(0);
> +}
> +}
> +
> +gicd = acpi_data_push(table_data, sizeof *gicd);
> +gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
> +gicd->length = sizeof(*gicd);
> +gicd->base_address = *info->gic_dist_base_addr;
> +
> +build_header(linker, table_data,
> + (void *)(table_data->data + madt_start), "APIC",
> + table_data->len - madt_start, 1);
> +}
> +
>  /* FADT */
>  static void
>  build_fadt(GArray *table_data, GArray *linker, unsigned dsdt)
> @@ -231,8 +286,11 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  {
>  GArray *table_offsets;
>  unsigned dsdt;
> +VirtAcpiCpuInfo cpuinfo;
>  GArray *tables_blob = tables->table_data;
>  
> +virt_acpi_get_cpu_info(&cpuinfo);
> +
>  table_offsets = g_array_new(false, true /* clear */,
>  sizeof(uint32_t));
>  
> @@ -257,6 +315,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  acpi_add_table(table_offsets, tables_blob);
>  build_fadt(tables_blob, tables->linker, dsdt);
>  
> +acpi_add_table(table_offsets, tables_blob);
> +build_madt(tables_blob, tables->linker, guest_info, &cpuinfo);
> +
>  /* Cleanup memory that's no longer used. */
>  g_array_free(table_offsets, true);
>  }
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index e588df5..e343728 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -235,7 +235,13 @@ typedef struct AcpiMultipleApicTable 
> AcpiMultipleApicTable;
>  #define ACPI_APIC_IO_SAPIC   6
>  #define ACPI_APIC_LOCAL_SAPIC7
>  #define ACPI_APIC_XRUPT_SOURCE   8
> -#define ACPI_APIC_RESERVED   9   /* 9 and greater are 
> reserved */
> +#define ACPI_APIC_LOCAL_X2APIC   9
> +#define ACPI_APIC_LOCAL_X2APIC_NMI  10
> +#define ACPI_APIC_GENERIC_INTERRUPT 11
> +#define ACPI_APIC_GENERIC_DISTRIBUTOR   12
> +#define ACPI_APIC_GENERIC_MSI_FRAME 13
> +#define ACPI_APIC_GENERIC_REDISTRIBUTOR 14
> +#define ACPI_APIC_RESERVED  15   /* 15 and greater are reserved 
> */
>  
>  /*
>   * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE)
> @@ -283,6 +289,34 @@ struct AcpiMadtLocalNmi {
>  } QEMU_PACKED;
>  typedef struct AcpiMadtLocalNmi AcpiMadtLocalNmi;
>  
> +struct AcpiMadtGenericInterrupt {
> +ACPI_SUB_HEADER_DEF
> +uint16_t reserved;
> +uint32_t cpu_interface_number;
> +uint32_t uid;
> +uint32_t flags;
> +  

Re: [Qemu-devel] [RFC PATCH 0/3] pflash_cfi01: allow reading/writing it only in secure mode

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 15:58, Edgar E. Iglesias wrote:
> Hi Paulo,
> 
> How would this work with XIP off the romd region?
> Without s/ns address spaces,  CPUs in NS state will be able to execute
> and access data while in ROMD state won't they?

Good point!  In fact, even with S/NS address spaces, the ROMD state is
global across all CPUs, so if one CPU does a secure write all other CPUs
would fail to access the ROM in non-secure mode.  Even if I modified
pflash_mem_read to return ROM contents, it would fail to execute.

This works for UEFI because the reset vector is the only executable code
in the flash.  The actual firmware volumes are compressed.

> I may be missing something...

You may also be missing (I didn't say it) that this is for x86 not ARM. :->

Paolo



Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 11:21, Paolo Bonzini  wrote:
>
>
> On 09/04/2015 12:14, Peter Maydell wrote:
>> On 9 April 2015 at 10:59, Edgar E. Iglesias  wrote:
>>> > On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote:
 >> Make address_space_rw take transaction attributes, rather
 >> than always using the 'unspecified' attributes.
>>> >
>>> > Reviewed-by: Edgar E. Iglesias 
>>> >
>>> > I guess that we eventually will need to convert the dma_
>>> > functions?
>> Probably, though I'm not clear what they bring to the party
>> that the basic address_space_* functions don't (part of why
>> I left them alone).
>
> At this point, some memory barriers, basically.

So what distinguishes a device that needs the memory barriers
and does its accesses via dma_* from a device that doesn't and
uses address_space_* or ld/st*_phys ? (Or for that matter a
non-device that does memory accesses...)

thanks
-- PMM



Re: [Qemu-devel] [PATCH 0/2] qom: strdup() target_name on object_property_add_alias()

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 21:57, Eduardo Habkost wrote:
> This helps us avoid memory leaks when using object_property_add_alias(), as it
> is not practical for callers to save target_name to free it later.
> 
> Eduardo Habkost (2):
>   qom: strdup() target property name on object_property_add_alias()
>   qdev: Free property names after registering gpio aliases
> 
>  hw/core/qdev.c | 2 ++
>  qom/object.c   | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 

Good idea!

Reviewed-by: Paolo Bonzini 



Re: [Qemu-devel] [PULL 0/4] Block patches

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 10:55, Stefan Hajnoczi  wrote:
> The following changes since commit 5a24f20a7208a58fb80d78ca0521bba6f4d7b145:
>
>   Merge remote-tracking branch 
> 'remotes/mjt/tags/pull-trivial-patches-2015-04-04' into staging (2015-04-07 
> 14:33:46 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 05b685fbabb7fdcab72cb42b27db916fd74b2265:
>
>   block/iscsi: handle zero events from iscsi_which_events (2015-04-09 
> 10:31:45 +0100)

Applied, thanks.

-- PMM



[Qemu-devel] [PATCH 2/2] arm_gicv2m: set kvm_gsi_direct_mapping and kvm_msi_via_irqfd_allowed

2015-04-09 Thread Eric Auger
After introduction of kvm_arch_msi_data_to_gsi, kvm_gsi_direct_mapping
now can be set on ARM. Also kvm_msi_via_irqfd_allowed can be set,
depending on kernel irqfd support, hence enabling VIRTIO-PCI with
vhost back-end.

Signed-off-by: Eric Auger 
---
 hw/intc/arm_gicv2m.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c
index a80a16d..7c6bea2 100644
--- a/hw/intc/arm_gicv2m.c
+++ b/hw/intc/arm_gicv2m.c
@@ -138,6 +138,8 @@ static void gicv2m_realize(DeviceState *dev, Error **errp)
 }
 
 msi_supported = true;
+kvm_gsi_direct_mapping = true;
+kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
 }
 
 static void gicv2m_init(Object *obj)
-- 
1.8.3.2




[Qemu-devel] [PATCH 1/2] kvm: introduce kvm_arch_msi_data_to_gsi

2015-04-09 Thread Eric Auger
On ARM the MSI data corresponds to the shared peripheral interrupt (SPI)
ID. This latter equals to the SPI index + 32. to retrieve the SPI index,
matching the gsi, an architecture specific function is introduced.

Signed-off-by: Eric Auger 
---
 include/sysemu/kvm.h | 2 ++
 kvm-all.c| 2 +-
 target-arm/kvm.c | 5 +
 target-i386/kvm.c| 5 +
 target-mips/kvm.c| 5 +
 target-ppc/kvm.c | 5 +
 target-s390x/kvm.c   | 5 +
 7 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 197e6c0..ebd8b17 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -286,6 +286,8 @@ void kvm_arch_init_irq_routing(KVMState *s);
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
  uint64_t address, uint32_t data);
 
+int kvm_arch_msi_data_to_gsi(uint32_t data);
+
 int kvm_set_irq(KVMState *s, int irq, int level);
 int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
 
diff --git a/kvm-all.c b/kvm-all.c
index dd44f8c..3e9675e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1228,7 +1228,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
 int virq;
 
 if (kvm_gsi_direct_mapping()) {
-return msg.data & 0x;
+return kvm_arch_msi_data_to_gsi(msg.data);
 }
 
 if (!kvm_gsi_routing_enabled()) {
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index fdd9ba3..05d5634 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -598,3 +598,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
*route,
 {
 return 0;
 }
+
+int kvm_arch_msi_data_to_gsi(uint32_t data)
+{
+return (data - 32) & 0x;
+}
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 41d09e5..7c648e7 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2764,3 +2764,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
*route,
 {
 return 0;
 }
+
+int kvm_arch_msi_data_to_gsi(uint32_t data)
+{
+return data & 0x;
+}
diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 4d1f7ea..e53e059 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -694,3 +694,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
*route,
 {
 return 0;
 }
+
+int kvm_arch_msi_data_to_gsi(uint32_t data)
+{
+return data & 0x;
+}
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 12328a4..53569f6 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2408,3 +2408,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
*route,
 {
 return 0;
 }
+
+int kvm_arch_msi_data_to_gsi(uint32_t data)
+{
+return data & 0x;
+}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b48c643..6509aff 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -1940,3 +1940,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry 
*route,
 route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
 return 0;
 }
+
+int kvm_arch_msi_data_to_gsi(uint32_t data)
+{
+return data & 0x;
+}
-- 
1.8.3.2




Re: [Qemu-devel] [PATCH v3] translate-all: use glib for all page descriptor allocations

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 22:07, Emilio G. Cota wrote:
> Since commit
> 
>   b7b5233a "bsd-user/mmap.c: Don't try to override g_malloc/g_free"
> 
> the exception we make here for usermode has been unnecessary.
> Get rid of it.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  translate-all.c | 18 ++
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/translate-all.c b/translate-all.c
> index 4d05898..acf792c 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -389,18 +389,6 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  void **lp;
>  int i;
>  
> -#if defined(CONFIG_USER_ONLY)
> -/* We can't use g_malloc because it may recurse into a locked mutex. */
> -# define ALLOC(P, SIZE) \
> -do {\
> -P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,\
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
> -} while (0)
> -#else
> -# define ALLOC(P, SIZE) \
> -do { P = g_malloc0(SIZE); } while (0)
> -#endif
> -
>  /* Level 1.  Always allocated.  */
>  lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
>  
> @@ -412,7 +400,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  if (!alloc) {
>  return NULL;
>  }
> -ALLOC(p, sizeof(void *) * V_L2_SIZE);
> +p = g_new0(void *, V_L2_SIZE);
>  *lp = p;
>  }
>  
> @@ -424,12 +412,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>  if (!alloc) {
>  return NULL;
>  }
> -ALLOC(pd, sizeof(PageDesc) * V_L2_SIZE);
> +pd = g_new0(PageDesc, V_L2_SIZE);
>  *lp = pd;
>  }
>  
> -#undef ALLOC
> -
>  return pd + (index & (V_L2_SIZE - 1));
>  }
>  
> 

Thanks, looks good.

Paolo



Re: [Qemu-devel] [PATCH v4 12/20] hw/arm/virt-acpi-build: Add PCIe info and generate MCFG table

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 16:54, Alex Bennée  wrote:
>
> Shannon Zhao  writes:
>
>> From: Shannon Zhao 
>> +build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>> +{
>> +AcpiTableMcfg *mcfg;
>> +acpi_pcie_info *info = guest_info->pcie_info;
>> +int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
>
> Explicit bracketing around the maths please.

This doesn't seem to make much sense anyway:
if the addition was intended to take precedence
then we're adding 1 to a size-of-a-struct, which is
a bit weird. And if the multiplication was intended
to take precedence then it's doing a pointless multiply
by one. Please can you check that this is actually
calculating the right value?

thanks
-- PMM



Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

2015-04-09 Thread Laszlo Ersek
On 04/09/15 15:59, Peter Maydell wrote:
> On 9 April 2015 at 14:51, Igor Mammedov  wrote:
>> On Thu, 9 Apr 2015 14:27:58 +0100
>> Peter Maydell  wrote:
>>
>>> On 9 April 2015 at 14:17, Igor Mammedov  wrote:
 On Thu, 09 Apr 2015 13:50:52 +0100
 Alex Bennée  wrote:

>
> Shannon Zhao  writes:
>> +for (i = 0; i < table_offsets->len; ++i) {
>> +/* rsdt->table_offset_entry to be filled by Guest linker */
>> +bios_linker_loader_add_pointer(linker,
>> +   ACPI_BUILD_TABLE_FILE,
>> +   ACPI_BUILD_TABLE_FILE,
>> +   table_data, 
>> &rsdt->table_offset_entry[i],
>> +   sizeof(uint32_t));
>
> Why are these pointers always 32 bit? Can they ever be 64 bit?
 Laszlo, can you confirm that UEFI puts APCI tables below 4G address
 space?

I confirmed that before, in the v2 discussion:

http://thread.gmane.org/gmane.comp.emulators.qemu/316670/focus=317560

But in fact the RSDT / XSDT that QEMU exports for UEFI doesn't matter.
See below.

>>> In the general case you can't guarantee that there will
>>> be any RAM at all below the 4G point. (The virt board
>>> isn't like that, obviously, but I believe there's real
>>> hardware out there that's designed that way.) I don't
>>> think we should have any 32 bit assumptions in the
>>> code at all -- pointer values should always be 64 bits
>>> everywhere.
>>
>> then that forces us to use xsdt instead of 32-bit rsdt
>
> Does that matter much?

I can mention two points here.


(1) See this kernel patch:

http://thread.gmane.org/gmane.linux.acpi.devel/74369/focus=1915858

> +The ACPI core will ignore any provided RSDT (Root System Description Table).
> +RSDTs have been deprecated and are ignored on arm64 since they only allow
> +for 32-bit addresses.

So you could argue that providing an XSDT instead of an RSDT is
justified by this alone.


(2) the ACPI linker/loader client in edk2 (used for both OVMF and AAVMF)
*does* restrict initial allocations to under 4GB. This is a super hairy
subject, but I'll try to summarize it quickly.

The ACPI linker/loader interface was originally designed for SeaBIOS.
The central structure of the interface is a command table, which
contains three kinds of commands:
(a) "allocate blob" (which includes "download blob" as well),
(b) "relocate pointer" (also known as "update pointer"), and
(c) "checksum blob segment".

Importantly, the "allocate" command comes with allocation hints; it can
instruct the firmware to allocate the blob in some specific zone.

So, the general process (as designed) is something like this:
- The firmware allocates and downloads the blobs -- the allocate
  commands always come first in the table. Each blob can contain several
  ACPI tables, in any kind of arrangement.

- Then the "relocate pointer" commands are processed (simply because
  they come later in the command table, that's guaranteed), which update
  pointers in some tables (residing in some blobs) to some other tables
  (residing in some other, or the same, blob(s)). In more detail, the
  pointers in the tables in the blobs are pre-initialized with relative
  offsets (into other blobs), and the relocation means that these
  relative offsets are made absolute -- they are incremented with the
  actual allocation base addresses that are the results of the
  allocation command processing (see previous step).

- Finally (in fact, intermixed with "relocate pointer" commands),
  checksums are updated.

The idea is that after the initial allocations, everything is processed
*in place*. (This is what SeaBIOS does.) Because pointer fields, updated
by the "relocate pointer" commands (which basically mean increments by
actual blob base addresses) can come in various sizes (1, 2, 4, 8
bytes), the allocation commands must take care to instruct the firmware
to allocate the "target" blobs "low enough" so that the referring
pointers can accommodate these actual base addresses.

All fine; again, SeaBIOS does exactly this; the important thing to note
is that everything is processed, and then left for the runtime OS, *in
place*.

And then UEFI / edk2 came along. :)

The problem with UEFI is that you are not supposed to just throw a bunch
of binary stuff into RAM. Instead, the RSD PTR table needs to be linked
into the UEFI system config table, plus each table needs to be installed
*individually*, by passing it to EFI_ACPI_TABLE_PROTOCOL.InstallTable().

The first requirement is actually a relaxation -- the RSD PTR can be
anywhere in memory, it doesn't need to be low. However, the second
requirement is a huge pain, because it doesn't match the design of the
ACPI linker/loader interface. EFI_ACPI_TABLE_PROTOCOL is "smart" about
the specification, and knows what to allocate where -- it copies tables,
links the copies together, checksums them, handles the RSD PTR
internally

Re: [Qemu-devel] seccomp breakage on arm

2015-04-09 Thread Paul Moore
On Thursday, April 09, 2015 02:28:24 PM Andreas Färber wrote:
> Am 09.04.2015 um 11:10 schrieb Paul Moore:
> > On Thursday, April 09, 2015 10:21:52 AM Eduardo Otubo wrote:
> >> On Thu, Apr 09, 2015 at 05=01=31AM +0200, Andreas Färber wrote:
> >>> Hello,
> >>> 
> >>> I am seeing the following build failure on openSUSE Tumbleweed armv7l
> >>> with --enable-seccomp in v2.3.0-rc2:
> >>> 
> >>> [  551s] In file included from qemu-seccomp.c:16:0:
> >>> [  551s] /usr/include/libseccomp/seccomp.h:177:23: error: '__NR_mmap'
> >>> undeclared here (not in a function)
> >>> [  551s]  #define SCMP_SYS(x)  (__NR_##x)
> >>> [  551s]^
> >>> [  551s] qemu-seccomp.c:36:7: note: in expansion of macro 'SCMP_SYS'
> >>> [  551s]  { SCMP_SYS(mmap), 247 },
> >>> [  551s]^
> >>> [  551s] /usr/include/libseccomp/seccomp.h:177:23: error:
> >>> '__NR_getrlimit' undeclared here (not in a function)
> >>> [  551s]  #define SCMP_SYS(x)  (__NR_##x)
> >>> [  551s]^
> >>> [  551s] qemu-seccomp.c:57:7: note: in expansion of macro 'SCMP_SYS'
> >>> [  551s]  { SCMP_SYS(getrlimit), 245 },
> >>> [  551s]^
> >>> [  551s] /home/abuild/rpmbuild/BUILD/qemu-2.3.0-rc2/rules.mak:57: recipe
> >>> for target 'qemu-seccomp.o' failed
> >>> [  551s] make: *** [qemu-seccomp.o] Error 1
> >>> 
> >>> Is this a problem with libseccomp 2.2.0 / master and needs to be fixed
> >>> in the library? Or do we need to #ifdef some syscalls in qemu-seccomp.c?
> >> 
> >> This should be already fixed in the library as mentioned by the
> >> maintainer in this[0] thread. Adding Paul Moore in CC. There's also a
> >> bug entry on launchpad[1] for that. I provided the patch (before the
> >> pull reuqest) requesting for some review and testing but never heard
> >> back again. Also CC'ing Karl-Philipp Richter (bug owner) for some
> >> opinions on that as well.
> >> 
> >> Regards,
> >> 
> >> [0] http://sourceforge.net/p/libseccomp/mailman/message/32955831/
> >> [1] https://bugs.launchpad.net/qemu/+bug/1363641
> > 
> > This should be fixed with libseccomp v2.2.0; if you are still seeing
> > problems using v2.2.0 let me know.
> 
> This *is* with libseccomp v2.2.0, as mentioned above, and I had checked
> that there were no related changes beyond v2.2.0 on your master branch.

I saw were you were *asking* if this was a problem with libseccomp v2.2.0, not 
stating that you were seeing a problem with v2.2.0; I interpreted your 
comments as running a version of libseccomp < v2.2.0 and you were asking if 
the problem had been fixed before your upgraded your copy of libseccomp.

Regardless, I think I see what the problem is, and if I'm correct it affects 
time, umount, stime, alarm, utime, getrlimit, select, readdir, mmap, 
socketcall, syscall, and ipc.  I'm traveling at the moment so a patch may be a 
bit delayed, but I'll be sure to CC you on the fix in case you are able to do 
some testing.

Thanks for the report, I'm sorry about the initial confusion.

-Paul

-- 
paul moore
security @ redhat




Re: [Qemu-devel] [PATCH] Fix crash with illegal "-net nic, model=xxx" option

2015-04-09 Thread Michael S. Tsirkin
On Thu, Apr 09, 2015 at 03:32:45PM +0200, Thomas Huth wrote:
> Current QEMU crashes when specifying an illegal model with the
> "-net nic,model=xxx" option, e.g.:
> 
>  $ qemu-system-x86_64 -net nic,model=n/a
>  qemu-system-x86_64: Unsupported NIC model: n/a
> 
>  Program received signal SIGSEGV, Segmentation fault.
> 
> The gdb backtrace looks like this:
> 
> 0x55965fe0 in error_get_pretty (err=0x0) at util/error.c:152
> 152   return err->msg;
> (gdb) bt
>  0  0x55965fe0 in error_get_pretty (err=0x0) at util/error.c:152
>  1  0x55965ffd in error_report_err (err=0x0) at util/error.c:157
>  2  0x55809c90 in pci_nic_init_nofail (nd=0x55e49860 , 
> rootbus=0x564409b0,
> default_model=0x5598c37b "e1000", default_devaddr=0x0) at 
> hw/pci/pci.c:1663
>  3  0x55691e42 in pc_nic_init (isa_bus=0x56f71900, 
> pci_bus=0x564409b0)
> at hw/i386/pc.c:1506
>  4  0x5569396b in pc_init1 (machine=0x562abbf0, pci_enabled=1, 
> kvmclock_enabled=1)
> at hw/i386/pc_piix.c:248
>  5  0x55693d27 in pc_init_pci (machine=0x562abbf0) at 
> hw/i386/pc_piix.c:310
>  6  0x5572ddf5 in main (argc=3, argv=0x7fffe018, 
> envp=0x7fffe038) at vl.c:4226
> 
> The problem is that pci_nic_init_nofail() does not check whether the err
> parameter from pci_nic_init has been set up and thus passes a NULL pointer
> to error_report_err(). Fix it by correctly checking the err parameter.
> 
> Signed-off-by: Thomas Huth 

Thanks!
Given that this is a legacy -net option, I'm inclined
to fix it post-2.3, and Cc stable.
Unfortunately I won't be able to do a pull request before rc3.

> ---
>  hw/pci/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6941a82..b3d5100 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1660,7 +1660,9 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
> *rootbus,
>  
>  res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
>  if (!res) {
> -error_report_err(err);
> +if (err) {
> +error_report_err(err);
> +}
>  exit(1);
>  }
>  return res;
> -- 
> 1.8.3.1



[Qemu-devel] [PATCH 08/10] s390x/kvm: Put vm name, extended name and UUID into STSI322 SYSIB

2015-04-09 Thread Cornelia Huck
From: Ekaterina Tumanova 

KVM prefills the SYSIB, returned by STSI 3.2.2. This patch allows
userspace to intercept execution, and fill in the values, that are
known to qemu: machine name (8 chars), extended machine name (256
chars), extended machine name encoding (equals 2 for UTF-8) and UUID.

STSI322 qemu handler also finds a highest virtualization level in
level-3 virtualization stack that doesn't support Extended Names
(Ext Name delimiter) and propagates zero Ext Name to all levels below,
because this level is not capable of managing Extended Names of lower
levels.

Signed-off-by: Ekaterina Tumanova 
Reviewed-by: Christian Borntraeger 
Reviewed-by: Thomas Huth 
Signed-off-by: Cornelia Huck 
---
 target-s390x/cpu.h |  8 --
 target-s390x/kvm.c | 71 ++
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8135dda..79bc80b 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -865,9 +865,13 @@ struct sysib_322 {
 uint8_t  name[8];
 uint32_t caf;
 uint8_t  cpi[16];
-uint8_t  res3[24];
+uint8_t res5[3];
+uint8_t ext_name_encoding;
+uint32_t res3;
+uint8_t uuid[16];
 } vm[8];
-uint8_t res4[3552];
+uint8_t res4[1504];
+uint8_t ext_names[8][256];
 };
 
 /* MMU defines */
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b48c643..619684b 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -44,6 +44,7 @@
 #include "hw/s390x/s390-pci-inst.h"
 #include "hw/s390x/s390-pci-bus.h"
 #include "hw/s390x/ipl.h"
+#include "hw/s390x/ebcdic.h"
 
 /* #define DEBUG_KVM */
 
@@ -255,6 +256,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 }
 
 kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
+kvm_vm_enable_cap(s, KVM_CAP_S390_USER_STSI, 0);
 
 return 0;
 }
@@ -1723,6 +1725,72 @@ static int handle_tsch(S390CPU *cpu)
 return ret;
 }
 
+static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr)
+{
+struct sysib_322 sysib;
+int del;
+
+if (s390_cpu_virt_mem_read(cpu, addr, &sysib, sizeof(sysib))) {
+return;
+}
+/* Shift the stack of Extended Names to prepare for our own data */
+memmove(&sysib.ext_names[1], &sysib.ext_names[0],
+sizeof(sysib.ext_names[0]) * (sysib.count - 1));
+/* First virt level, that doesn't provide Ext Names delimits stack. It is
+ * assumed it's not capable of managing Extended Names for lower levels.
+ */
+for (del = 1; del < sysib.count; del++) {
+if (!sysib.vm[del].ext_name_encoding || !sysib.ext_names[del][0]) {
+break;
+}
+}
+if (del < sysib.count) {
+memset(sysib.ext_names[del], 0,
+   sizeof(sysib.ext_names[0]) * (sysib.count - del));
+}
+/* Insert short machine name in EBCDIC, padded with blanks */
+if (qemu_name) {
+memset(sysib.vm[0].name, 0x40, sizeof(sysib.vm[0].name));
+ebcdic_put(sysib.vm[0].name, qemu_name, MIN(sizeof(sysib.vm[0].name),
+strlen(qemu_name)));
+}
+sysib.vm[0].ext_name_encoding = 2; /* 2 = UTF-8 */
+memset(sysib.ext_names[0], 0, sizeof(sysib.ext_names[0]));
+/* If hypervisor specifies zero Extended Name in STSI322 SYSIB, it's
+ * considered by s390 as not capable of providing any Extended Name.
+ * Therefore if no name was specified on qemu invocation, we go with the
+ * same "KVMguest" default, which KVM has filled into short name field.
+ */
+if (qemu_name) {
+strncpy((char *)sysib.ext_names[0], qemu_name,
+sizeof(sysib.ext_names[0]));
+} else {
+strcpy((char *)sysib.ext_names[0], "KVMguest");
+}
+/* Insert UUID */
+memcpy(sysib.vm[0].uuid, qemu_uuid, sizeof(sysib.vm[0].uuid));
+
+s390_cpu_virt_mem_write(cpu, addr, &sysib, sizeof(sysib));
+}
+
+static int handle_stsi(S390CPU *cpu)
+{
+CPUState *cs = CPU(cpu);
+struct kvm_run *run = cs->kvm_run;
+
+switch (run->s390_stsi.fc) {
+case 3:
+if (run->s390_stsi.sel1 != 2 || run->s390_stsi.sel2 != 2) {
+return 0;
+}
+/* Only sysib 3.2.2 needs post-handling for now. */
+insert_stsi_3_2_2(cpu, run->s390_stsi.addr);
+return 0;
+default:
+return 0;
+}
+}
+
 static int kvm_arch_handle_debug_exit(S390CPU *cpu)
 {
 CPUState *cs = CPU(cpu);
@@ -1772,6 +1840,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run 
*run)
 case KVM_EXIT_S390_TSCH:
 ret = handle_tsch(cpu);
 break;
+case KVM_EXIT_S390_STSI:
+ret = handle_stsi(cpu);
+break;
 case KVM_EXIT_DEBUG:
 ret = kvm_arch_handle_debug_exit(cpu);
 break;
-- 
2.3.5




[Qemu-devel] [PATCH 03/10] s390-virtio: sort into categories

2015-04-09 Thread Cornelia Huck
Sort the various s390-virtio devices into the same categories as their
virtio-pci counterparts.

Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/s390-virtio-bus.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 047c963..c211da4 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -519,6 +519,7 @@ static void s390_virtio_net_class_init(ObjectClass *klass, 
void *data)
 
 k->realize = s390_virtio_net_realize;
 dc->props = s390_virtio_net_properties;
+set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
 }
 
 static const TypeInfo s390_virtio_net = {
@@ -532,8 +533,10 @@ static const TypeInfo s390_virtio_net = {
 static void s390_virtio_blk_class_init(ObjectClass *klass, void *data)
 {
 VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
 
 k->realize = s390_virtio_blk_realize;
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
 static const TypeInfo s390_virtio_blk = {
@@ -555,6 +558,7 @@ static void s390_virtio_serial_class_init(ObjectClass 
*klass, void *data)
 
 k->realize = s390_virtio_serial_realize;
 dc->props = s390_virtio_serial_properties;
+set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
 static const TypeInfo s390_virtio_serial = {
@@ -577,6 +581,7 @@ static void s390_virtio_rng_class_init(ObjectClass *klass, 
void *data)
 
 k->realize = s390_virtio_rng_realize;
 dc->props = s390_virtio_rng_properties;
+set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }
 
 static const TypeInfo s390_virtio_rng = {
@@ -635,6 +640,7 @@ static void s390_virtio_scsi_class_init(ObjectClass *klass, 
void *data)
 
 k->realize = s390_virtio_scsi_realize;
 dc->props = s390_virtio_scsi_properties;
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
 static const TypeInfo s390_virtio_scsi = {
@@ -658,6 +664,7 @@ static void s390_vhost_scsi_class_init(ObjectClass *klass, 
void *data)
 
 k->realize = s390_vhost_scsi_realize;
 dc->props = s390_vhost_scsi_properties;
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
 static const TypeInfo s390_vhost_scsi = {
@@ -681,8 +688,10 @@ static int s390_virtio_bridge_init(SysBusDevice *dev)
 static void s390_virtio_bridge_class_init(ObjectClass *klass, void *data)
 {
 SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
 
 k->init = s390_virtio_bridge_init;
+set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
 static const TypeInfo s390_virtio_bridge_info = {
-- 
2.3.5




Re: [Qemu-devel] [PATCH v2 3/4] target-i386: Register QOM properties for feature flags

2015-04-09 Thread Eduardo Habkost
On Wed, Apr 08, 2015 at 04:02:42PM -0300, Eduardo Habkost wrote:
[...]
> +names = g_strsplit(fi->feat_names[bit], "|", 0);

I forgot to implement the property aliases Igor asked for. I will submit
v3 of this patch later.

> +for (i = 0; names[i]; i++) {
> +char *feat_name = names[i];
> +feat2prop(feat_name);
> +char *prop_name = g_strdup_printf("cpuid-%s", feat_name);
> +x86_cpu_register_feature_prop(cpu, prop_name, w, (1UL << bit));
> +g_free(prop_name);
> +}
> +g_strfreev(names);
> +}
> +
-- 
Eduardo



Re: [Qemu-devel] [PATCH 0/2] MAINTAINERS: X86 update

2015-04-09 Thread Paolo Bonzini


On 08/04/2015 18:57, Eduardo Habkost wrote:
> Add myself as X86 maintainer, and change its status to "Maintained".
> 
> Eduardo Habkost (2):
>   MAINTAINERS: Add myself to X86
>   MAINTAINERS: Change status of X86 to Maintained
> 
>  MAINTAINERS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

x86 TCG would still be in odd fixes mode, but it's not worth splitting
the stanza.

Acked-by: Paolo Bonzini 

Paolo



Re: [Qemu-devel] [RFC PATCH 0/3] pflash_cfi01: allow reading/writing it only in secure mode

2015-04-09 Thread Peter Maydell
On 9 April 2015 at 13:20, Paolo Bonzini  wrote:
> This is an example of usage of attributes in a device model.  It lets
> you block flash writes unless the CPU is in secure mode.  Enabling it
> currently requires a -readconfig file:
>
> [global]
> driver = "cfi.pflash01"
> property = "secure"
> value = "on"
>
> because the driver includes a "."; however, I plan to enable this through
> the command line for the final version of the patches.

Are real flash devices ever wired up like this?
I would expect boards which want to provide secure-mode
only flash to do so by not giving any access at all to
the device from the non-secure address space.

(Supporting multiple AddressSpaces for ARM CPUs is the
next thing on my todo list; as well as partitioning the
flash this would allow secure-mode-only RAM and UARTs,
for instance.)

-- PMM



Re: [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes

2015-04-09 Thread Paolo Bonzini


On 09/04/2015 12:14, Peter Maydell wrote:
> On 9 April 2015 at 10:59, Edgar E. Iglesias  wrote:
>> > On Tue, Apr 07, 2015 at 09:09:52PM +0100, Peter Maydell wrote:
>>> >> Make address_space_rw take transaction attributes, rather
>>> >> than always using the 'unspecified' attributes.
>> >
>> > Reviewed-by: Edgar E. Iglesias 
>> >
>> > I guess that we eventually will need to convert the dma_
>> > functions?
> Probably, though I'm not clear what they bring to the party
> that the basic address_space_* functions don't (part of why
> I left them alone).

At this point, some memory barriers, basically.

Initially the IOMMU implementation was done in the dma_* functions
(using something called IIRC a DMAContext), but now they just take an
AddressSpace.

Paolo



Re: [Qemu-devel] [PATCH v4 10/20] hw/arm/virt-acpi-build: Generate RSDT table

2015-04-09 Thread Igor Mammedov
On Thu, 9 Apr 2015 14:59:09 +0100
Peter Maydell  wrote:

> On 9 April 2015 at 14:51, Igor Mammedov  wrote:
> > On Thu, 9 Apr 2015 14:27:58 +0100
> > Peter Maydell  wrote:
> >
> >> On 9 April 2015 at 14:17, Igor Mammedov  wrote:
> >> > On Thu, 09 Apr 2015 13:50:52 +0100
> >> > Alex Bennée  wrote:
> >> >
> >> >>
> >> >> Shannon Zhao  writes:
> >> >> > +for (i = 0; i < table_offsets->len; ++i) {
> >> >> > +/* rsdt->table_offset_entry to be filled by Guest linker */
> >> >> > +bios_linker_loader_add_pointer(linker,
> >> >> > +   ACPI_BUILD_TABLE_FILE,
> >> >> > +   ACPI_BUILD_TABLE_FILE,
> >> >> > +   table_data, 
> >> >> > &rsdt->table_offset_entry[i],
> >> >> > +   sizeof(uint32_t));
> >> >>
> >> >> Why are these pointers always 32 bit? Can they ever be 64 bit?
> >> > Laszlo, can you confirm that UEFI puts APCI tables below 4G address 
> >> > space?
> >>
> >> In the general case you can't guarantee that there will
> >> be any RAM at all below the 4G point. (The virt board
> >> isn't like that, obviously, but I believe there's real
> >> hardware out there that's designed that way.) I don't
> >> think we should have any 32 bit assumptions in the
> >> code at all -- pointer values should always be 64 bits
> >> everywhere.
> >
> > then that forces us to use xsdt instead of 32-bit rsdt
> 
> Does that matter much?
not much, using rsdt would allow to share this code with x86.

also having tables below 4Gb in rsdt would make life of
32 bit guests easier, not that there are such guests now and may be
there wouldn't be any.

> 
> -- PMM
> 




Re: [Qemu-devel] [PATCH v4 11/20] hw/arm/virt-acpi-build: Generate RSDP table

2015-04-09 Thread Alex Bennée

Shannon Zhao  writes:

> From: Shannon Zhao 
>
> RSDP points to RSDT which in turn points to other tables.
>
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Alex Bennée 

> ---
>  hw/arm/virt-acpi-build.c | 35 ++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 85e84b1..dd5538b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -176,6 +176,35 @@ static void acpi_dsdt_add_virtio(Aml *scope, const 
> hwaddr *mmio_addrs,
>  }
>  }
>  
> +/* RSDP */
> +static GArray *
> +build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> +{
> +AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
> +
> +bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
> + true /* fseg memory */);
> +
> +memcpy(&rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
> +memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
> +rsdp->length = cpu_to_le32(sizeof(*rsdp));
> +rsdp->revision = 0x02;
> +
> +/* Point to RSDT */
> +rsdp->rsdt_physical_address = cpu_to_le32(rsdt);
> +/* Address to be filled by Guest linker */
> +bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE,
> +   ACPI_BUILD_TABLE_FILE,
> +   rsdp_table, &rsdp->rsdt_physical_address,
> +   sizeof rsdp->rsdt_physical_address);
> +rsdp->checksum = 0;
> +/* Checksum to be filled by Guest linker */
> +bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> +rsdp, rsdp, sizeof *rsdp, 
> &rsdp->checksum);
> +
> +return rsdp_table;
> +}
> +
>  /* RSDT */
>  static void
>  build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
> @@ -336,7 +365,7 @@ static
>  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>  {
>  GArray *table_offsets;
> -unsigned dsdt;
> +unsigned dsdt, rsdt;
>  VirtAcpiCpuInfo cpuinfo;
>  GArray *tables_blob = tables->table_data;
>  
> @@ -373,8 +402,12 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  build_gtdt(tables_blob, tables->linker, guest_info);
>  
>  /* RSDT is pointed to by RSDP */
> +rsdt = tables_blob->len;
>  build_rsdt(tables_blob, tables->linker, table_offsets);
>  
> +/* RSDP is in FSEG memory, so allocate it separately */
> +build_rsdp(tables->rsdp, tables->linker, rsdt);
> +
>  /* Cleanup memory that's no longer used. */
>  g_array_free(table_offsets, true);
>  }

-- 
Alex Bennée



  1   2   >