Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional
> 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
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
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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