Re: [Qemu-devel] [RFC v2 3/4] hw/pci-host: Add a generic PCI host controller for virtual platforms
Hi,
On Mon, Jan 5, 2015 at 6:13 PM, Alexander Graf wrote:
>
>
> On 21.11.14 19:07, Alvise Rigo wrote:
>> Add a generic PCI host controller for virtual platforms, based on the
>> previous work by Rob Herring:
>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
>>
>> The controller creates a PCI bus for PCI devices; it is intended to
>> receive from the platform all the needed information to initiate the
>> memory regions expected by the PCI system. Due to this dependence, a
>> header file is used to define the structure that the platform has to
>> fill with the data needed by the controller. The device provides also a
>> routine for the device tree node generation, which mostly has to take
>> care of the interrupt-map property generation. This property is fetched
>> by the guest operating system to map any PCI interrupt to the interrupt
>> controller. For the time being, the device expects a GIC v2 to be used
>> by the guest.
>> Only mach-virt has been used to test the controller.
>>
>> Signed-off-by: Alvise Rigo
>> ---
>> hw/pci-host/Makefile.objs | 2 +-
>> hw/pci-host/generic-pci.c | 280
>> ++
>> include/hw/pci-host/generic-pci.h | 74 ++
>> 3 files changed, 355 insertions(+), 1 deletion(-)
>> create mode 100644 hw/pci-host/generic-pci.c
>> create mode 100644 include/hw/pci-host/generic-pci.h
>>
>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>> index bb65f9c..8ef9fac 100644
>> --- a/hw/pci-host/Makefile.objs
>> +++ b/hw/pci-host/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -common-obj-y += pam.o
>> +common-obj-y += pam.o generic-pci.o
>>
>> # PPC devices
>> common-obj-$(CONFIG_PREP_PCI) += prep.o
>> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
>> new file mode 100644
>> index 000..9c90263
>> --- /dev/null
>> +++ b/hw/pci-host/generic-pci.c
>> @@ -0,0 +1,280 @@
>> +/*
>> + * Generic PCI host controller
>> + *
>> + * Copyright (c) 2014 Linaro, Ltd.
>> + * Author: Rob Herring
>> + *
>> + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
>> + * Copyright (c) 2006-2009 CodeSourcery.
>> + * Written by Paul Brook
>> + *
>> + * This code is licensed under the LGPL.
>> + */
>> +
>> +#include "hw/sysbus.h"
>> +#include "hw/pci-host/generic-pci.h"
>> +#include "exec/address-spaces.h"
>> +#include "sysemu/device_tree.h"
>> +
>> +static const VMStateDescription pci_generic_host_vmstate = {
>> +.name = "generic-host-pci",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +};
>> +
>> +static void pci_cam_config_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned size)
>> +{
>> +PCIGenState *s = opaque;
>> +pci_data_write(&s->pci_bus, addr, val, size);
>> +}
>> +
>> +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned
>> size)
>> +{
>> +PCIGenState *s = opaque;
>> +uint32_t val;
>> +val = pci_data_read(&s->pci_bus, addr, size);
>> +return val;
>> +}
>> +
>> +static const MemoryRegionOps pci_vpb_config_ops = {
>> +.read = pci_cam_config_read,
>> +.write = pci_cam_config_write,
>> +.endianness = DEVICE_NATIVE_ENDIAN,
>
> Never use NATIVE_ENDIAN unless you're 100% sure it's correct. In this
> case, please make it LITTLE_ENDIAN.
Agreed.
>
>> +};
>> +
>> +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
>> +{
>> +qemu_irq *pic = opaque;
>> +qemu_set_irq(pic[irq_num], level);
>> +}
>> +
>> +static void pci_generic_host_init(Object *obj)
>> +{
>> +PCIHostState *h = PCI_HOST_BRIDGE(obj);
>> +PCIGenState *s = PCI_GEN(obj);
>> +GenericPCIHostState *gps = &s->pci_gen;
>> +
>> +memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
>
> Why is IO space that big? It's only 64k usually, no?
This was just to prevent a definition of the io space smaller than the
actual size of the memory region.
This could be avoided as you suggested later, by postponing the
creation of the PCIBus.
>
>> +memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>> +
>> +pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), "pci",
>> +&s->pci_mem_space, &s->pci_io_space,
>> +PCI_DEVFN(0, 0), TYPE_PCIE_BUS);
>> +h->bus = &s->pci_bus;
>> +
>> +object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
>> +qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
>> +gps->devfns = 0;
>> +}
>> +
>> +static int generic_pci_map_irq_fn(PCIDevice *pci_dev, int pin)
>> +{
>> +BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
>> +PCIBus *pci_bus = PCI_BUS(bus);
>> +PCIDevice *pdev = pci_bus->devices[PCI_DEVFN(0, 0)];
>> +GenericPCIHostState *gps = PCI_GEN_HOST(pdev);
>> +
>> +return gps->irqmap.devfn_idx_map[PCI_SLOT(pci_dev->devfn)]
>> +[PCI_FUNC(pci_dev->devfn)];
>> +}
>> +
>> +static void
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 2015/1/6 14:20, Shannon Zhao wrote:
On 2015/1/6 10:37, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
qemu_machine_opts global list") removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState->usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum
Cc: Tiejun Chen
Signed-off-by: Stefan Hajnoczi
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
- has_defaults && default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
+return current_machine->usb;
+} else {
+return has_defaults && default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks for your test.
Tiejun
Thanks,
Shannon
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
return desc->def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt->desc) {
desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
str = g_strdup(desc->def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const
char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_number(name, desc->def_value_str, &ret,
&error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_size(name, desc->def_value_str, &ret, &error_abort);
Re: [Qemu-devel] [RFC v2 1/4] hw/arm/virt: Allow multiple agents to modify dt
On 01/05/2015 05:14 PM, alvise rigo wrote: > Hi, > > On Mon, Jan 5, 2015 at 4:36 PM, Peter Maydell > wrote: >> On 24 November 2014 at 11:47, Claudio Fontana >> wrote: >>> On 21.11.2014 19:07, Alvise Rigo wrote: Keep a global list with all the functions that need to modify the device tree. Using qemu_add_machine_init_done_notifier we register a notifier that executes all the functions on the list and loads the kernel. Signed-off-by: Alvise Rigo >>> >>> Peter, could you weigh in about whether this is a good idea or not? >> >> Sorry, I think I must have missed this series first time around. >> I'm not convinced -- I don't see any reason why we should treat >> the PCI host controller differently from other devices in the > > The reason for this is that the PCI host controller needs to generate > its device node after all the PCI devices have been added to the bus > (also those specified with the -device option). > This is required by the interrupt-map node property, that specifies > for each PCI device an interrupt map entry. Since we have one device > requiring this 'postponed' node generation, this patch allows also > other devices to do the same. > Recently I was thinking to another approach, which is to have multiple > dtb modifiers called by arm_boot_info.modify_dtb(). Is this > reasonable? > >> virt board: just generate an appropriate dtb fragment in virt.c. > > Of course, the method that generates the device node fragment can be > moved to virt.c, but the requirement of postponing its call after the > machine has been created still applies. Hi Alvise, Peter, Besides the PCI aspects, the dt generation problem that is addressed here is identical to the one related to VFIO platform device dt node generation that also needs to happen after machine init. In "machvirt dynamic sysbus device instantiation" (https://www.mail-archive.com/qemu-devel@nongnu.org/msg272506.html), arm_load_kernel is proposed to be executed in a machine init done notifier and VFIO node creation happens in another machine init done notifier registered after this latter. Best Regards Eric > > Thank you for your feedback, > alvise > >> >> thanks >> -- PMM >
Re: [Qemu-devel] [RFC v2 1/4] hw/arm/virt: Allow multiple agents to modify dt
Hi Eric, You are right. In fact, I've also spent some time to see if it was possible to use the code you mentioned. However, it's not needed anymore: the node generation will happen at machine init for the reasons discussed in this thread. Regards, alvise On Tue, Jan 6, 2015 at 10:18 AM, Eric Auger wrote: > On 01/05/2015 05:14 PM, alvise rigo wrote: >> Hi, >> >> On Mon, Jan 5, 2015 at 4:36 PM, Peter Maydell >> wrote: >>> On 24 November 2014 at 11:47, Claudio Fontana >>> wrote: On 21.11.2014 19:07, Alvise Rigo wrote: > Keep a global list with all the functions that need to modify the device > tree. Using qemu_add_machine_init_done_notifier we register a notifier > that executes all the functions on the list and loads the kernel. > > Signed-off-by: Alvise Rigo Peter, could you weigh in about whether this is a good idea or not? >>> >>> Sorry, I think I must have missed this series first time around. >>> I'm not convinced -- I don't see any reason why we should treat >>> the PCI host controller differently from other devices in the >> >> The reason for this is that the PCI host controller needs to generate >> its device node after all the PCI devices have been added to the bus >> (also those specified with the -device option). >> This is required by the interrupt-map node property, that specifies >> for each PCI device an interrupt map entry. Since we have one device >> requiring this 'postponed' node generation, this patch allows also >> other devices to do the same. >> Recently I was thinking to another approach, which is to have multiple >> dtb modifiers called by arm_boot_info.modify_dtb(). Is this >> reasonable? >> >>> virt board: just generate an appropriate dtb fragment in virt.c. >> >> Of course, the method that generates the device node fragment can be >> moved to virt.c, but the requirement of postponing its call after the >> machine has been created still applies. > > Hi Alvise, Peter, > > Besides the PCI aspects, the dt generation problem that is addressed > here is identical to the one related to VFIO platform device dt node > generation that also needs to happen after machine init. > > In "machvirt dynamic sysbus device instantiation" > (https://www.mail-archive.com/qemu-devel@nongnu.org/msg272506.html), > arm_load_kernel is proposed to be executed in a machine init done > notifier and VFIO node creation happens in another machine init done > notifier registered after this latter. > > Best Regards > > Eric >> >> Thank you for your feedback, >> alvise >> >>> >>> thanks >>> -- PMM >> >
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On 6 January 2015 at 00:22, Programmingkid wrote: > http://virtuallyfun.superglobalmegacorp.com/?p=3197 > This is how Mac OS X looks like in QEMU with a Mac OS X host. The colors are > all wrong. Right, so that says there is a bug somewhere. But this patch isn't fixing a bug, it's adding a command line switch. > Your option 1. QEMU does not emulate any video card that shipped > with a Mac. So we can't use this option. We *must* be emulating a video card, otherwise we would not be displaying anything! > Option 2. I don't know what you mean by asking the host's GUI > about it. I do know that having the user choosing the right > option does work. Yes, but it's basically making the user manually toggle a setting which we should be getting right ourselves. We should find out what QEMU's actually not doing correctly and fix that. thanks -- PMM
Re: [Qemu-devel] [RFC v2 1/4] hw/arm/virt: Allow multiple agents to modify dt
On 6 January 2015 at 09:18, Eric Auger wrote: > Besides the PCI aspects, the dt generation problem that is addressed > here is identical to the one related to VFIO platform device dt node > generation that also needs to happen after machine init. Right, for VFIO we need it; but for PCI we don't, so we shouldn't tangle the two up together unnecessarily. -- PMM
Re: [Qemu-devel] [PATCH v2 01/10] pci: move REDHAT_SDHCI device ID to make room for Rocker
On 6 January 2015 at 02:24, wrote: > From: Scott Feldman > > The rocker device uses same PCI device ID as sdhci. Since rocker device > driver > has already been accepted into Linux 3.18, and REDHAT_SDHCI device ID isn't > used by any drivers, it's safe to move REDHAT_SDHCI device ID, avoiding > conflict with rocker. Same remarks apply as I made on v1 of this patch -- I don't want to take this series until we have an answer for who's the authoritative source for handing out IDs in this space and why we ended up with this conflict. thanks -- PMM
Re: [Qemu-devel] [RFC v2 1/4] hw/arm/virt: Allow multiple agents to modify dt
Thank you. I will keep this in mind for the next spin of the patches. Regards, alvise On Mon, Jan 5, 2015 at 7:07 PM, Peter Maydell wrote: > On 5 January 2015 at 17:35, alvise rigo wrote: >> So I suppose we need to define a fixed number of PCI slots according >> to the number of interrupts available in mach-virt. In this regard, >> should this number be a qdev property? > > The PCI spec means that a bus has an implicit maximum of > 32 slots (the devfn in a PCI address is only 5 bits). > Note that this doesn't imply having 32 interrupt lines. > > What you can do is something like this (which is what the > versatilepb device-tree-binding will have): > > + interrupt-map-mask = <0x1800 0 0 7>; > + interrupt-map = <0x1800 0 0 1 &sic 28 > +0x1800 0 0 2 &sic 29 > +0x1800 0 0 3 &sic 30 > +0x1800 0 0 4 &sic 27 > + > +0x1000 0 0 1 &sic 27 > +0x1000 0 0 2 &sic 28 > +0x1000 0 0 3 &sic 29 > +0x1000 0 0 4 &sic 30 > + > +0x0800 0 0 1 &sic 30 > +0x0800 0 0 2 &sic 27 > +0x0800 0 0 3 &sic 28 > +0x0800 0 0 4 &sic 29 > + > +0x 0 0 1 &sic 29 > +0x 0 0 2 &sic 30 > +0x 0 0 3 &sic 27 > +0x 0 0 4 &sic 28>; > > That says "we have four interrupts, which are swizzled in > the usual way between slots", and doesn't impose any > particular maximum number of slots. (Notice that the > mask value is 0x1800 0 0 0 7, which means we only match > on the low two bits of the devfn, so a unit-interrupt-specifier > of <0x2000 0x0 0x0 1> for slot 4 matches the entry > <0x 0x0 0x0 1> in the map table, as required.) > > (You'll want to do something more sensible than 27..30, > which is just what the interrupt lines on the versatilepb > happen to be.) > > -- PMM
Re: [Qemu-devel] [RFC PATCH v3 2/3] hw/arm/virt: Don't add memory node in creat_fdt
On 6 January 2015 at 05:57, Shannon Zhao wrote: > To support memory NUMA, don't add memory node in creat_fdt. > But add it in a new function which takes into accout NUMA > topology. > > Signed-off-by: Shannon Zhao > --- > hw/arm/virt.c |2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index fdafa79..505cd29 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -190,8 +190,6 @@ static void create_fdt(VirtBoardInfo *vbi) > * to fill in necessary properties later > */ > qemu_fdt_add_subnode(fdt, "/chosen"); > -qemu_fdt_add_subnode(fdt, "/memory"); > -qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory"); > > /* Clock node, for the benefit of the UART. The kernel device tree > * binding documentation claims the PL011 node clock properties are This patch will break bisection of all platforms using device tree; you need to keep things working at all points in your patch series, not just at the end when all the patches are applied. thanks -- PMM
Re: [Qemu-devel] [RFC PATCH v3 3/3] hw/arm/boot: Generate memory dtb according to NUMA topology
On 6 January 2015 at 05:57, Shannon Zhao wrote: > Add a new function arm_generate_memory_dtb which is used to > generate memory dtb according to NUMA topology and set the > NUMA topology property of every cpu. > > Signed-off-by: Shannon Zhao This looks a lot nicer than the previous patchset. It doesn't seem to me like there's much point in reviewing it in detail until the kernel/dtb folks have confirmed the dtb bindings for NUMA and committed the documentation into the kernel tree, though. thanks -- PMM
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On 6 January 2015 at 09:47, Peter Maydell wrote: > Yes, but it's basically making the user manually toggle a > setting which we should be getting right ourselves. We > should find out what QEMU's actually not doing correctly > and fix that. First step to find out what's happening: which of the following combinations give the messed up colours? * OSX host, Linux guest [no, at least for me] * OSX host, OSX guest [yes] * Linux host, Linux guest [no] * Linux host, OSX guest ??? Secondly, are we talking about specifically x86 OSX host, or both x86 and PPC OSX host, or just PPC OSX host? And is the OSX guest x86, ppc or both? thanks -- PMM
Re: [Qemu-devel] [RFC v2 1/4] hw/arm/virt: Allow multiple agents to modify dt
On 01/06/2015 10:51 AM, Peter Maydell wrote: > On 6 January 2015 at 09:18, Eric Auger wrote: >> Besides the PCI aspects, the dt generation problem that is addressed >> here is identical to the one related to VFIO platform device dt node >> generation that also needs to happen after machine init. > > Right, for VFIO we need it; but for PCI we don't, so we shouldn't > tangle the two up together unnecessarily. OK understood Eric > > -- PMM >
Re: [Qemu-devel] [RFC PATCH v3 2/3] hw/arm/virt: Don't add memory node in creat_fdt
On 2015/1/6 17:55, Peter Maydell wrote: > On 6 January 2015 at 05:57, Shannon Zhao wrote: >> To support memory NUMA, don't add memory node in creat_fdt. >> But add it in a new function which takes into accout NUMA >> topology. >> >> Signed-off-by: Shannon Zhao >> --- >> hw/arm/virt.c |2 -- >> 1 files changed, 0 insertions(+), 2 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index fdafa79..505cd29 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -190,8 +190,6 @@ static void create_fdt(VirtBoardInfo *vbi) >> * to fill in necessary properties later >> */ >> qemu_fdt_add_subnode(fdt, "/chosen"); >> -qemu_fdt_add_subnode(fdt, "/memory"); >> -qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory"); >> >> /* Clock node, for the benefit of the UART. The kernel device tree >> * binding documentation claims the PL011 node clock properties are > > This patch will break bisection of all platforms using device tree; > you need to keep things working at all points in your patch series, > not just at the end when all the patches are applied. > Hi Peter, Thanks for your remind. I'll make the patch 2 and 3 together in next version. Thanks, Shannon
Re: [Qemu-devel] [RFC PATCH v3 3/3] hw/arm/boot: Generate memory dtb according to NUMA topology
On 2015/1/6 17:58, Peter Maydell wrote: > On 6 January 2015 at 05:57, Shannon Zhao wrote: >> Add a new function arm_generate_memory_dtb which is used to >> generate memory dtb according to NUMA topology and set the >> NUMA topology property of every cpu. >> >> Signed-off-by: Shannon Zhao > > This looks a lot nicer than the previous patchset. It > doesn't seem to me like there's much point in reviewing it > in detail until the kernel/dtb folks have confirmed the > dtb bindings for NUMA and committed the documentation into > the kernel tree, though. > Good :) I will also stay tuned on that. Thanks, Shannon
Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1
Comments inline
-Original Message-
From: Jeff Cody [mailto:jc...@redhat.com]
Sent: Tuesday, December 23, 2014 7:33 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kw...@redhat.com; stefa...@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to
bdrv_has_zero_init_1
On Tue, Dec 23, 2014 at 05:07:16AM -0500, Lokesha, Amulya wrote:
>
> -Original Message-
> From: Jeff Cody [mailto:jc...@redhat.com]
> Sent: Wednesday, December 17, 2014 5:44 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kw...@redhat.com;
> stefa...@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to
> bdrv_has_zero_init_1
>
> On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> > Hi Max, Jeff,
> >
> > We were able to get the qemu patch files downloaded from the qemu patch
> > site - https://patchwork.ozlabs.org and were able to apply the patches
> > successfully without any errors. With the patches applied, we recompiled
> > the qemu and converted the VDK vmdk to vhdx format and uploaded to the
> > SCVMM Server. But it failed again with the syntax error as below:
> >
> > Information (10804)
> > Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx
> > because of a syntax error in the file.
> >
> > Please find my comments inline for your questions
> >
> > Please let us know if there is anything else you need from us.
> >
> >
>
> Amulya,
>
> I will try to test this on Windows Server, and see if I can reproduce what
> you are seeing.
>
> -Jeff
>
>
> Hi Jeff,
>
> Any updates on this? Were you able to test it
>
> Thanks,
> Amulya
>
Still in process. I am working to get some MSDN licensing issues resolved, and
then I will be able to test.
Hi Jeff,
Were you able to get it tested? We are waiting to deliver the patches to our
customers. Please let us know
Thanks,
Amulya
> >
> >
> > -Original Message-
> > From: Jeff Cody [mailto:jc...@redhat.com]
> > Sent: Friday, December 12, 2014 8:48 PM
> > To: Lokesha, Amulya
> > Cc: Max Reitz; qemu-devel@nongnu.org; kw...@redhat.com;
> > stefa...@redhat.com
> > Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to
> > bdrv_has_zero_init_1
> >
> > On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> > >Hi Max,
> > >
> > >
> >
> > Please reply in-line, it makes it easier to follow technical
> > discussions - thanks :)
> >
> > >
> > >We applied all the 5 patches from the mail chain I got since the last
> > >week. Please find attached the patches used by us.
> > >
> > >We were unable to apply the patch3 as it failed with the
> > > following error
> > >
> > >
> > >
> > ># patch -p1 < patch3
> > >
> > >patching file block/vhdx.c
> > >
> > >patch: malformed patch at line 17: error_setg_errno(errp,
> > >EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@
> > > static
> > >QemuOptsList vhdx_create_opts = {
> > >
> > >
> >
> > It looks like however you saved the patch file, it was corrupted.
> > Looking at your attached patch 3, it split line 9 across 2 lines.
> > Your patch also has whitespace differences from the patch I sent.
> >
> > You also attached 5 patches - Why are you using patch 0? You should only
> > be applying patches 1-4. This should not be causing any actual issues,
> > however.
> > [Amulya]: First time we applied patches 1 to 4, created VHDX image and
> > deployed to HyperV Server, but we got the same error. Then we took a fresh
> > qemu source and applied patches 0 to 4 and deployed to HyperV and again got
> > the same syntax error.
> >
> >
> > Are you using git for your qemu version? If so, 'git am' is the preferred
> > method of applying the patches - just save each of the patch emails (the
> > whole email should be fine), and run 'git am' on each file.
> >
> > [Amulya] : No. We don't have a git repository for our team. Could you
> > please let us know how to apply these patches without git. What is the
> > difference in applying the patch directly and modifying the code directly?
> > Does it have any impact?
> >
> > >
> > >
> > >
> > >Hence, we manually added the patch3 changes and recompiled the qemu. We
> > >then used the patched qemu-img to convert our vmdk image to dynamic
> > > VHDX
> > >format. We found that the image created this time had a considerable
> > >decrease in its size from 50GB to 12GB.
> > >
> >
> > Could you tell me the file size of the VMDK image you were converting?
> > Is it roughly 12GB as well?
> > [Amulya] : No, the vmdk image which we used for conversion is just
> > 1.4GB
> >
> >
> > >However, when we deployed it into our SCVMM 2012, the import of the
> > > VHDX
> > >image failed with a "syntax error" as below
> > >
> > >
> > >
> > >Information (10804)
> > >
> > >Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx bec
Re: [Qemu-devel] [PULL 09/10] monitor: add query-vnc2 command
> > Add new query vnc qmp command, for the lack of better ideas just name it > > "query-vnc2". Changes over query-vnc: > Call it query-vnc-servers? Fine with me, done. > Maybe just VncPrimaryAuth, since there's precedence for abbreviating > authentication that way. Done. > > ## > > +# @query-vnc2: > > +# > > +# Returns a list of vnc servers. The list can be empty. > > +# > > +# Returns: @VncInfo > > Returns: a list of @VncInfo2 Yep. While being at it and as we've dropped the '2' from the command name: Suggestions for a better struct name? Note there already is a VncServerInfo struct, so going for something like VncServersInfo looks like a bad idea ... > > +case VNC_AUTH_SASL: > > +info->auth = VNC_PRI_AUTH_SASL; > > +break; > > +case VNC_AUTH_NONE: > > +default: > > +info->auth = VNC_PRI_AUTH_NONE; > > +break; > > +} > > T-e-d-i-o-u-s :) > > What about mapping vnc.h's authenticaton modes to the QAPI enumeration > constants with tables rather than switches? Your choice. Did it this way due to the numbers being sparse. cheers, Gerd
Re: [Qemu-devel] [PULL 09/10] monitor: add query-vnc2 command
Hi,
> > If the schema let us specify the enumeration values, we could avoid the
> > mapping altogether.
>
> Ooh, cool thought: what if we allowed:
>
> { 'enum': 'Foo', 'data': [
> { 'name': 'One', 'value': 1 },
> { 'name': 'Two', 'value': 2 } ] }
Looks nice. Where is the patch?
cheers,
Gerd
Re: [Qemu-devel] [Xen-devel] bind interdomain ioctl error xen-kvm.c
On Mon, 2015-01-05 at 11:10 -0800, Rishi Ranjan wrote: > Hi Stefano, > Please find my answers inline. > > > However Anthony (CC'ed) should have some patches for it. > > Anthony, can you please share any patch that can help me with this? > > > > > Can you post the full output of the logs? > I have attached the output of "sudo xl -v create /etc/xen/qemu-pv.cfg" > as xl_create.txt. I have also enabled DEBUG_XEN_HVM in xen-hvm.c and > pasted output of "sudo ./x86_64-softmmu/qemu-system-x86_64 -machine > q35,accel=xen -cpu qemu64 -xen-domid 13" below: Have you done anything which stops "xl create" from also launching a qemu? The guest cfg file you posted earlier didn't suggest so. Running two qemu's against the same domain doesn't seem likely to result in good things... > Did you execute the xencommons init script at boot time? > On Ubuntu I don't see /etc/init.d/xencommon but there is > a /etc/init.d/xen script which starts xenstored and xenconsoled. Yes, Debian and Ubuntu don't use the upstream initscript but have their own. Ian.
Re: [Qemu-devel] bind interdomain ioctl error xen-kvm.c
On Mon, Jan 05, 2015 at 11:10:34AM -0800, Rishi Ranjan wrote: > However Anthony (CC'ed) should have some patches for it. > Anthony, can you please share any patch that can help me with this? Hi, The "patches" are in two repos: git://xenbits.xen.org/people/aperard/xen-unstable.git branch: machine-q35-wip git://xenbits.xen.org/people/aperard/qemu-dm.git branch: xen-q35-wip For the xen tree, it's the 3 top commit, and the top commit for qemu. Once applied, hvmloader will only work with Q35 and not with the default machine. -- Anthony PERARD
Re: [Qemu-devel] [RFC v2 3/4] hw/pci-host: Add a generic PCI host controller for virtual platforms
On 06.01.15 09:47, alvise rigo wrote:
> Hi,
>
> On Mon, Jan 5, 2015 at 6:13 PM, Alexander Graf wrote:
>>
>>
>> On 21.11.14 19:07, Alvise Rigo wrote:
>>> Add a generic PCI host controller for virtual platforms, based on the
>>> previous work by Rob Herring:
>>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
>>>
>>> The controller creates a PCI bus for PCI devices; it is intended to
>>> receive from the platform all the needed information to initiate the
>>> memory regions expected by the PCI system. Due to this dependence, a
>>> header file is used to define the structure that the platform has to
>>> fill with the data needed by the controller. The device provides also a
>>> routine for the device tree node generation, which mostly has to take
>>> care of the interrupt-map property generation. This property is fetched
>>> by the guest operating system to map any PCI interrupt to the interrupt
>>> controller. For the time being, the device expects a GIC v2 to be used
>>> by the guest.
>>> Only mach-virt has been used to test the controller.
>>>
>>> Signed-off-by: Alvise Rigo
>>> ---
>>> hw/pci-host/Makefile.objs | 2 +-
>>> hw/pci-host/generic-pci.c | 280
>>> ++
>>> include/hw/pci-host/generic-pci.h | 74 ++
>>> 3 files changed, 355 insertions(+), 1 deletion(-)
>>> create mode 100644 hw/pci-host/generic-pci.c
>>> create mode 100644 include/hw/pci-host/generic-pci.h
>>>
>>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>>> index bb65f9c..8ef9fac 100644
>>> --- a/hw/pci-host/Makefile.objs
>>> +++ b/hw/pci-host/Makefile.objs
>>> @@ -1,4 +1,4 @@
>>> -common-obj-y += pam.o
>>> +common-obj-y += pam.o generic-pci.o
>>>
>>> # PPC devices
>>> common-obj-$(CONFIG_PREP_PCI) += prep.o
>>> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
>>> new file mode 100644
>>> index 000..9c90263
>>> --- /dev/null
>>> +++ b/hw/pci-host/generic-pci.c
>>> @@ -0,0 +1,280 @@
>>> +/*
>>> + * Generic PCI host controller
>>> + *
>>> + * Copyright (c) 2014 Linaro, Ltd.
>>> + * Author: Rob Herring
>>> + *
>>> + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
>>> + * Copyright (c) 2006-2009 CodeSourcery.
>>> + * Written by Paul Brook
>>> + *
>>> + * This code is licensed under the LGPL.
>>> + */
>>> +
>>> +#include "hw/sysbus.h"
>>> +#include "hw/pci-host/generic-pci.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "sysemu/device_tree.h"
>>> +
>>> +static const VMStateDescription pci_generic_host_vmstate = {
>>> +.name = "generic-host-pci",
>>> +.version_id = 1,
>>> +.minimum_version_id = 1,
>>> +};
>>> +
>>> +static void pci_cam_config_write(void *opaque, hwaddr addr,
>>> + uint64_t val, unsigned size)
>>> +{
>>> +PCIGenState *s = opaque;
>>> +pci_data_write(&s->pci_bus, addr, val, size);
>>> +}
>>> +
>>> +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned
>>> size)
>>> +{
>>> +PCIGenState *s = opaque;
>>> +uint32_t val;
>>> +val = pci_data_read(&s->pci_bus, addr, size);
>>> +return val;
>>> +}
>>> +
>>> +static const MemoryRegionOps pci_vpb_config_ops = {
>>> +.read = pci_cam_config_read,
>>> +.write = pci_cam_config_write,
>>> +.endianness = DEVICE_NATIVE_ENDIAN,
>>
>> Never use NATIVE_ENDIAN unless you're 100% sure it's correct. In this
>> case, please make it LITTLE_ENDIAN.
>
> Agreed.
>
>>
>>> +};
>>> +
>>> +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
>>> +{
>>> +qemu_irq *pic = opaque;
>>> +qemu_set_irq(pic[irq_num], level);
>>> +}
>>> +
>>> +static void pci_generic_host_init(Object *obj)
>>> +{
>>> +PCIHostState *h = PCI_HOST_BRIDGE(obj);
>>> +PCIGenState *s = PCI_GEN(obj);
>>> +GenericPCIHostState *gps = &s->pci_gen;
>>> +
>>> +memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
>>
>> Why is IO space that big? It's only 64k usually, no?
>
> This was just to prevent a definition of the io space smaller than the
> actual size of the memory region.
> This could be avoided as you suggested later, by postponing the
> creation of the PCIBus.
>
>>
>>> +memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL <<
>>> 32);
>>> +
>>> +pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj),
>>> "pci",
>>> +&s->pci_mem_space, &s->pci_io_space,
>>> +PCI_DEVFN(0, 0), TYPE_PCIE_BUS);
>>> +h->bus = &s->pci_bus;
>>> +
>>> +object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
>>> +qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
>>> +gps->devfns = 0;
>>> +}
>>> +
>>> +static int generic_pci_map_irq_fn(PCIDevice *pci_dev, int pin)
>>> +{
>>> +BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
>>> +PCIBus *pci_bus = PCI_BUS(bus);
>>> +PCIDevice *pdev = pci_bus->devices[PCI_DEVFN(0, 0)];
>>> +GenericPCIH
Re: [Qemu-devel] bind interdomain ioctl error xen-kvm.c
On Tue, Jan 06, 2015 at 11:11:48AM +, Anthony PERARD wrote: > On Mon, Jan 05, 2015 at 11:10:34AM -0800, Rishi Ranjan wrote: > > However Anthony (CC'ed) should have some patches for it. > > Anthony, can you please share any patch that can help me with this? > > Hi, > > The "patches" are in two repos: > git://xenbits.xen.org/people/aperard/xen-unstable.git > branch: machine-q35-wip > > git://xenbits.xen.org/people/aperard/qemu-dm.git > branch: xen-q35-wip > > For the xen tree, it's the 3 top commit, and the top commit for qemu. > Once applied, hvmloader will only work with Q35 and not with the > default machine. To start a guest, I have this in the guest config: disk = [ 'phy:/dev/mapper/vg-guest_q35,sda,w' ] machine='q35' If you wish, you can recompile only hvmloader and qemu and have something like this in your guest config: firmware_override = '/root/hvmloader-q35' device_model_override = '/root/qemu-xen-q35' device_model_version = 'qemu-xen' device_model_args_hvm = [ '-machine', 'q35,accel=xen' ] Hope that help. -- Anthony PERARD
Re: [Qemu-devel] [PATCH v2] libqos: Convert malloc-pc allocator to a generic allocator
On Thu, Oct 23, 2014 at 10:12:42AM +0200, Marc Marí wrote: > The allocator in malloc-pc has been extracted, so it can be used in every > arch. > This operation showed that both the alloc and free functions can be also > generic. > Because of this, the QGuestAllocator has been removed from is function to wrap > the alloc and free function, and now just contains the allocator parameters. > As a result, only the allocator initalizer and unitializer are arch dependent. > > Signed-off-by: Marc Marí > Reviewed-by: Stefan Hajnoczi > --- > tests/Makefile |2 +- > tests/libqos/malloc-pc.c | 280 > +- > tests/libqos/malloc-pc.h | 11 +- > tests/libqos/malloc.c| 270 > tests/libqos/malloc.h| 45 +--- > 5 files changed, 309 insertions(+), 299 deletions(-) > create mode 100644 tests/libqos/malloc.c Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpLTP9_c5P8Y.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 0/5] libqos: Virtio MMIO driver
On Wed, Dec 03, 2014 at 10:31:32AM +0100, Marc Marí wrote: > Add virtio-mmio support to libqos and test case for virtio-blk. > > This series depends on patch "libqos: Convert malloc-pc allocator to a > generic > allocator" > > Changes from version 2: > - Fix leaks and minor bugs > - Extract basic test case to a function > > Marc Marí (5): > libqos: Change use of pointers to uint64_t in virtio > tests: Prepare virtio-blk-test for multi-arch implementation > libqos: Remove PCI assumptions in constants of virtio driver > libqos: Add malloc generic > libqos: Add virtio MMIO support > > tests/Makefile|4 +- > tests/libqos/malloc-generic.c | 50 + > tests/libqos/malloc-generic.h | 21 > tests/libqos/virtio-mmio.c| 190 +++ > tests/libqos/virtio-mmio.h| 46 > tests/libqos/virtio-pci.c | 50 + > tests/libqos/virtio-pci.h | 24 ++-- > tests/libqos/virtio.c |8 +- > tests/libqos/virtio.h | 16 +-- > tests/virtio-blk-test.c | 249 > - > 10 files changed, 534 insertions(+), 124 deletions(-) > create mode 100644 tests/libqos/malloc-generic.c > create mode 100644 tests/libqos/malloc-generic.h > create mode 100644 tests/libqos/virtio-mmio.c > create mode 100644 tests/libqos/virtio-mmio.h > > -- > 1.7.10.4 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpLRTCpU64dD.pgp Description: PGP signature
Re: [Qemu-devel] 2.1 unexpected stop after exporting blockdev via nbd server
On Mon, Jan 05, 2015 at 05:12:10PM +0400, Andrey Korolyov wrote:
> On Fri, Jan 2, 2015 at 4:04 PM, Stefan Hajnoczi wrote:
> > On Thu, Dec 18, 2014 at 07:38:59PM +0400, Andrey Korolyov wrote:
> >> 2.1-stable is currently crashing with the
> >>
> >> Co-routine re-entered recursively
> >> 2014-12-16 15:06:23.578+: shutting down
> >>
> >> after execution of (for example) following when using virtio-dp as a
> >> disk backend:
> >>
> >> '{ "execute": "nbd-server-start", "arguments": { "addr": { "type":
> >> "inet", "data": { "host": "10.6.0.1", "port": "" } } } }'
> >> '{ "execute": "nbd-server-add", "arguments": {"device":
> >> "drive-virtio-disk0","writable": false } }'
> >
> > Hi,
> > You need patches from Max Reitz that make the run-time NBD server
> > support dataplane:
> > f214928 nbd: Follow the BDS' AIO context
> > 3338442 block: Add AIO context notifiers
> > 958c717 nbd: Drop nbd_can_read()
> >
> > Please try QEMU v2.2.0 or qemu.git.
> >
> > Stefan
>
> Thanks Stefan, those commits are enough to make dataplane drive work
> well with runtime nbd server. By the way, can such functionality
> improvements (not major improvements like discard support for scsi)
> make their way as backports? To reword this, unexpected behavior like
> sudden shutdown I described is better to be closed by later fixes than
> by stubs or left as it currently behaving IMO.
It is a bug that NBD combined with virtio-blk dataplane aborted, it
should have produced an error message and simply refused to start the
NBD server.
Now that QEMU 2.2 stable has been released I'm not sure if additional
2.1.x releases will be made at all. I have CCed qemu-stab...@nongnu.org
to see if there is interest.
Stefan
pgpgwOFrRJcO0.pgp
Description: PGP signature
Re: [Qemu-devel] [Bug 1404278] Re: tap connections not working on windows host
On Mon, Jan 05, 2015 at 05:11:50PM -, timsoft wrote: > have used wireshark on host and nothing is coming through when I try to ping > the host from the client. (bare with me as I haven't used wireshark before). > I'm just upgrading the client to slack64 14.1 so I can get wireshark > running on it. (process is a little slow, especially with no functioning > network on the client). If all goes well, I'll be able to test that by > tomorrow (it took about 5hours to install last time). I'll then post back > here. > If there are any specific tests or methods I could follow that would help > please let me know. Please post the following output from the guest: # ip addr # ip route # iptables -L -n This way we can verify that the guest's network interface is configured, up, and has no firewall rules that could filter packets. Please post output from the "ipconfig /all" command on the Windows host. Stefan pgpZFqEYvoHZC.pgp Description: PGP signature
Re: [Qemu-devel] 2.1 unexpected stop after exporting blockdev via nbd server
On Mon, Jan 05, 2015 at 05:12:10PM +0400, Andrey Korolyov wrote:
> On Fri, Jan 2, 2015 at 4:04 PM, Stefan Hajnoczi wrote:
> > On Thu, Dec 18, 2014 at 07:38:59PM +0400, Andrey Korolyov wrote:
> >> 2.1-stable is currently crashing with the
> >>
> >> Co-routine re-entered recursively
> >> 2014-12-16 15:06:23.578+: shutting down
> >>
> >> after execution of (for example) following when using virtio-dp as a
> >> disk backend:
> >>
> >> '{ "execute": "nbd-server-start", "arguments": { "addr": { "type":
> >> "inet", "data": { "host": "10.6.0.1", "port": "" } } } }'
> >> '{ "execute": "nbd-server-add", "arguments": {"device":
> >> "drive-virtio-disk0","writable": false } }'
> >
> > Hi,
> > You need patches from Max Reitz that make the run-time NBD server
> > support dataplane:
> > f214928 nbd: Follow the BDS' AIO context
> > 3338442 block: Add AIO context notifiers
> > 958c717 nbd: Drop nbd_can_read()
> >
> > Please try QEMU v2.2.0 or qemu.git.
> >
> > Stefan
>
> Thanks Stefan, those commits are enough to make dataplane drive work
> well with runtime nbd server. By the way, can such functionality
> improvements (not major improvements like discard support for scsi)
> make their way as backports? To reword this, unexpected behavior like
> sudden shutdown I described is better to be closed by later fixes than
> by stubs or left as it currently behaving IMO.
Oops, I typoed the qemu-sta...@nongnu.org email address. Trying
again...
QEMU stable folks: will there be another 2.1.x release?
Stefan
pgp8GJQqTRNgH.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option
On Sun, Dec 21, 2014 at 07:17:42AM +, Roy Vardi wrote: > When using "-net tap" flag today, the tap interface is not released on > shutdown. This is the default behavior and I'm not breaking it. ... > . /qemu-system -net tap,ifname=tap0,script=no,downscript=no That is not the behavior that I observe with Linux 3.17.4-302.fc21.x86_64 and qemu.git/master (ab0302ee764fd702465aef6d88612cdff4302809). When I run your command-line, the tap0 interface is visible. After QEMU shuts down the tap interface is no longer visible in the output of the "ip addr" command. What do you observe on your machine that makes you say "the tap interface is not released on shutdown"? Stefan pgp7tMtcCXg0L.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] net: remove all cleanup methods from NIC NetClientInfos
On Fri, Jan 02, 2015 at 05:22:27PM +0100, Paolo Bonzini wrote: > > > On 02/01/2015 15:00, Stefan Hajnoczi wrote: > >>> This cleanup function gets in the way of making the > >>> NetClientStates for the NIC hold an object_ref reference to > >>> the object, so get rid of it. > > This patch does not drop NetClientInfo->cleanup() and clean up > > net.c. Do you have plans for a follow-up patch? > > No, it is still in use by backends in net/. Only NICs do not need it. Good point, not sure how I missed that. Stefan pgpY5GSKdh958.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] net: remove all cleanup methods from NIC NetClientInfos
On Tue, Dec 23, 2014 at 05:53:19PM +0100, Paolo Bonzini wrote: > All NICs have a cleanup function that, in most cases, zeroes the pointer > to the NICState. In some cases, it frees data belonging to the NIC. > > However, this function is never called except when exiting from QEMU. > It is not necessary to NULL pointers and free data here; the right place > to do that would be in the device's unrealize function, after calling > qemu_del_nic. Zeroing the NIC multiple times is also wrong for multiqueue > devices. > > This cleanup function gets in the way of making the NetClientStates for > the NIC hold an object_ref reference to the object, so get rid of it. > > Signed-off-by: Paolo Bonzini > --- > hw/net/allwinner_emac.c | 8 > hw/net/cadence_gem.c| 9 - > hw/net/dp8393x.c| 11 --- > hw/net/e1000.c | 9 - > hw/net/eepro100.c | 8 > hw/net/etraxfs_eth.c| 13 - > hw/net/fsl_etsec/etsec.c| 6 -- > hw/net/lan9118.c| 8 > hw/net/lance.c | 8 > hw/net/mcf_fec.c| 8 > hw/net/milkymist-minimac2.c | 8 > hw/net/mipsnet.c| 8 > hw/net/ne2000-isa.c | 8 > hw/net/ne2000.c | 8 > hw/net/opencores_eth.c | 5 - > hw/net/pcnet-pci.c | 8 > hw/net/pcnet.c | 5 - > hw/net/pcnet.h | 1 - > hw/net/rtl8139.c| 8 > hw/net/smc91c111.c | 8 > hw/net/spapr_llan.c | 8 > hw/net/stellaris_enet.c | 8 > hw/net/virtio-net.c | 8 > hw/net/vmxnet3.c| 7 --- > hw/net/xgmac.c | 8 > hw/net/xilinx_axienet.c | 9 - > hw/net/xilinx_ethlite.c | 8 > 27 files changed, 211 deletions(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan pgptTlmo4VYbL.pgp Description: PGP signature
[Qemu-devel] [PATCH 3/6] hw/usb: simplified usb_enabled
The argument is not longer used and the implementation
uses now QOM instead of QemuOpts.
Signed-off-by: Marcel Apfelbaum
---
hw/arm/nseries.c| 2 +-
hw/arm/pxa2xx.c | 4 ++--
hw/arm/realview.c | 2 +-
hw/arm/versatilepb.c| 2 +-
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c| 2 +-
hw/ppc/mac_newworld.c | 2 +-
hw/ppc/mac_oldworld.c | 2 +-
hw/ppc/prep.c | 2 +-
hw/ppc/spapr.c | 2 +-
include/sysemu/sysemu.h | 2 +-
vl.c| 11 +--
12 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index c7ebaa6..4d7be5e 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1344,7 +1344,7 @@ static void n8x0_init(MachineState *machine,
n8x0_dss_setup(s);
n8x0_cbus_setup(s);
n8x0_uart_setup(s);
-if (usb_enabled(false)) {
+if (usb_enabled()) {
n8x0_usb_setup(s);
}
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 8967cc4..165ba2a 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -2143,7 +2143,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space,
s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi");
}
-if (usb_enabled(false)) {
+if (usb_enabled()) {
sysbus_create_simple("sysbus-ohci", 0x4c00,
qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1));
}
@@ -2276,7 +2276,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space,
unsigned int sdram_size)
s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi");
}
-if (usb_enabled(false)) {
+if (usb_enabled()) {
sysbus_create_simple("sysbus-ohci", 0x4c00,
qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1));
}
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 66e51ef..50cb93d 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -261,7 +261,7 @@ static void realview_init(MachineState *machine,
sysbus_connect_irq(busdev, 2, pic[50]);
sysbus_connect_irq(busdev, 3, pic[51]);
pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
-if (usb_enabled(false)) {
+if (usb_enabled()) {
pci_create_simple(pci_bus, -1, "pci-ohci");
}
n = drive_get_max_bus(IF_SCSI);
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 6c4c2e7..b1dae77 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -281,7 +281,7 @@ static void versatile_init(MachineState *machine, int
board_id)
pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);
}
}
-if (usb_enabled(false)) {
+if (usb_enabled()) {
pci_create_simple(pci_bus, -1, "pci-ohci");
}
n = drive_get_max_bus(IF_SCSI);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 220f741..f0a3201 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -274,7 +274,7 @@ static void pc_init1(MachineState *machine,
pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order,
machine, floppy, idebus[0], idebus[1], rtc_state);
-if (pci_enabled && usb_enabled(false)) {
+if (pci_enabled && usb_enabled()) {
pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7ba0535..a432944 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -265,7 +265,7 @@ static void pc_q35_init(MachineState *machine)
ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports);
ahci_ide_create_devs(ahci, hd);
-if (usb_enabled(false)) {
+if (usb_enabled()) {
/* Should we create 6 UHCI according to ich9 spec? */
ehci_create_ich9_with_companions(host_bus, 0x1d);
}
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 8ba9499..ed37d6b 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -418,7 +418,7 @@ static void ppc_core99_init(MachineState *machine)
qdev_init_nofail(dev);
if ((machine_arch == ARCH_MAC99_U3 && defaults_enabled()) ||
-usb_enabled(false)) {
+usb_enabled()) {
pci_create_simple(pci_bus, -1, "pci-ohci");
/* U3 needs to use USB for input because Linux doesn't support via-cuda
on PPC64 */
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index c7224d7..3079510 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -304,7 +304,7 @@ static void ppc_heathrow_init(MachineState *machine)
dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
qdev_init_nofail(dev);
-if (usb_enabled(false)) {
+if (usb_enabled()) {
pci_create_simple(pci_bus, -1, "pci-ohci");
}
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index dd8433d..15df7f3 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -539,7 +539,7 @@ static void ppc_prep_init(MachineState *machine)
memory_region_add_subregion(sysmem, 0xFEFF, xcsr);
#endif
-if (usb_enabled(false)) {
+if (usb_enabled()) {
[Qemu-devel] [PATCH 0/6] simplify usb enabling logic and fix a Qemu crash
Patch e79d5a6 ("machine: remove qemu_machine_opts global list") removed option
descriptions from the -machine QemuOptsList to avoid repeating MachineState's
QOM properties.
This resulted in a Qemu crash:
$ qemu-system-x86_64 -usb
qemu-system-x86_64: util/qemu-option.c:387: qemu_opt_get_bool_helper:
Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
Aborted (core dumped)
Fixed it by simplifying usb_enabled usage:
- removed the usb_enabled argument
- call directly to machine's QOM property.
- expose defaults_enabled to calling sites
As part of this series the semantics for some ppc machines to
create the default usb controller were changed, but I still think
is correct, please let me know if you see a problem there.
Based on the comments I will receive, I will continue to fix the options
properties in the same way.
Marcel Apfelbaum (6):
hw/ppc: modified the condition for usb controllers to be created for
some ppc machines
hw/machine: added machine_usb wrapper
hw/usb: simplified usb_enabled
hw/ppc/mac_newworld: QOMified mac99 machines
hw/ppc/spapr: simplify usb controller creation logic
hw/ppc/mac_newworld: simplify usb controller creation logic
hw/arm/nseries.c| 2 +-
hw/arm/pxa2xx.c | 4 ++--
hw/arm/realview.c | 2 +-
hw/arm/versatilepb.c| 2 +-
hw/core/machine.c | 5 +
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c| 2 +-
hw/ppc/mac_newworld.c | 32 +---
hw/ppc/mac_oldworld.c | 2 +-
hw/ppc/prep.c | 2 +-
hw/ppc/spapr.c | 3 ++-
include/hw/boards.h | 2 ++
include/sysemu/sysemu.h | 3 ++-
vl.c| 16 ++--
14 files changed, 51 insertions(+), 28 deletions(-)
--
2.1.0
[Qemu-devel] [PATCH 6/6] hw/ppc/mac_newworld: simplify usb controller creation logic
Signed-off-by: Marcel Apfelbaum
---
hw/ppc/mac_newworld.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index b54f94a..c377012 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -371,6 +371,7 @@ static void ppc_core99_init(MachineState *machine)
/* 970 gets a U3 bus */
pci_bus = pci_pmac_u3_init(pic, get_system_memory(), get_system_io());
machine_arch = ARCH_MAC99_U3;
+machine->usb |= defaults_enabled();
} else {
pci_bus = pci_pmac_init(pic, get_system_memory(), get_system_io());
machine_arch = ARCH_MAC99;
@@ -417,8 +418,7 @@ static void ppc_core99_init(MachineState *machine)
dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
qdev_init_nofail(dev);
-if ((machine_arch == ARCH_MAC99_U3 && defaults_enabled()) ||
-usb_enabled()) {
+if (machine->usb) {
pci_create_simple(pci_bus, -1, "pci-ohci");
/* U3 needs to use USB for input because Linux doesn't support via-cuda
on PPC64 */
--
2.1.0
[Qemu-devel] [PATCH 2/6] hw/machine: added machine_usb wrapper
Following QOM convention, object properties should
not be accessed directly.
Signed-off-by: Marcel Apfelbaum
---
hw/core/machine.c | 5 +
include/hw/boards.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index a0ae5f9..fbd91be 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -398,6 +398,11 @@ static void machine_finalize(Object *obj)
g_free(ms->firmware);
}
+bool machine_usb(MachineState *machine)
+{
+return machine->usb;
+}
+
static const TypeInfo machine_info = {
.name = TYPE_MACHINE,
.parent = TYPE_OBJECT,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index e0a6790..3ddc449 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -65,6 +65,8 @@ int qemu_register_machine(QEMUMachine *m);
MachineClass *find_default_machine(void);
extern MachineState *current_machine;
+bool machine_usb(MachineState *machine);
+
/**
* MachineClass:
* @qemu_machine: #QEMUMachine
--
2.1.0
[Qemu-devel] [PATCH 1/6] hw/ppc: modified the condition for usb controllers to be created for some ppc machines
Some ppc machines create a default usb controller based on a 'machine
condition'.
Until now the logic was: create the usb controller if:
- the usb option was supplied in cli and value is true or
- the usb option was absent and both set_defaults and the machine
condition were true.
Modified the logic to:
Create the usb controller if:
- the machine condition is true and defaults are enabled or
- the usb option is supplied and true.
The main for this is to simplify the usb_enabled method.
Signed-off-by: Marcel Apfelbaum
---
hw/ppc/mac_newworld.c | 3 ++-
hw/ppc/spapr.c | 2 +-
include/sysemu/sysemu.h | 1 +
vl.c| 7 ++-
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index b60a832..8ba9499 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -417,7 +417,8 @@ static void ppc_core99_init(MachineState *machine)
dev = qdev_create(adb_bus, TYPE_ADB_MOUSE);
qdev_init_nofail(dev);
-if (usb_enabled(machine_arch == ARCH_MAC99_U3)) {
+if ((machine_arch == ARCH_MAC99_U3 && defaults_enabled()) ||
+usb_enabled(false)) {
pci_create_simple(pci_bus, -1, "pci-ohci");
/* U3 needs to use USB for input because Linux doesn't support via-cuda
on PPC64 */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 08401e0..d5de301 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1486,7 +1486,7 @@ static void ppc_spapr_init(MachineState *machine)
spapr->has_graphics = true;
}
-if (usb_enabled(spapr->has_graphics)) {
+if ((spapr->has_graphics && defaults_enabled()) || usb_enabled(false)) {
pci_create_simple(phb->bus, -1, "pci-ohci");
if (spapr->has_graphics) {
usbdevice_create("keyboard");
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 503e5a4..a31044c 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -227,6 +227,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
QemuOpts *qemu_get_machine_opts(void);
+bool defaults_enabled(void);
bool usb_enabled(bool default_usb);
extern QemuOptsList qemu_legacy_drive_opts;
diff --git a/vl.c b/vl.c
index bea9656..415535f 100644
--- a/vl.c
+++ b/vl.c
@@ -997,10 +997,15 @@ static int parse_name(QemuOpts *opts, void *opaque)
return 0;
}
+bool defaults_enabled(void)
+{
+return has_defaults;
+}
+
bool usb_enabled(bool default_usb)
{
return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
- has_defaults && default_usb);
+ default_usb);
}
#ifndef _WIN32
--
2.1.0
[Qemu-devel] [PATCH 5/6] hw/ppc/spapr: simplify usb controller creation logic
Signed-off-by: Marcel Apfelbaum
---
hw/ppc/spapr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 72c3102..53c4116 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1484,9 +1484,10 @@ static void ppc_spapr_init(MachineState *machine)
/* Graphics */
if (spapr_vga_init(phb->bus)) {
spapr->has_graphics = true;
+machine->usb |= defaults_enabled();
}
-if ((spapr->has_graphics && defaults_enabled()) || usb_enabled()) {
+if (machine->usb) {
pci_create_simple(phb->bus, -1, "pci-ohci");
if (spapr->has_graphics) {
usbdevice_create("keyboard");
--
2.1.0
[Qemu-devel] [PATCH 4/6] hw/ppc/mac_newworld: QOMified mac99 machines
Signed-off-by: Marcel Apfelbaum
---
hw/ppc/mac_newworld.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index ed37d6b..b54f94a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -502,18 +502,27 @@ static int core99_kvm_type(const char *arg)
return 2;
}
-static QEMUMachine core99_machine = {
-.name = "mac99",
-.desc = "Mac99 based PowerMAC",
-.init = ppc_core99_init,
-.max_cpus = MAX_CPUS,
-.default_boot_order = "cd",
-.kvm_type = core99_kvm_type,
+static void core99_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->name = "mac99";
+mc->desc = "Mac99 based PowerMAC";
+mc->init = ppc_core99_init;
+mc->max_cpus = MAX_CPUS;
+mc->default_boot_order = "cd";
+mc->kvm_type = core99_kvm_type;
+}
+
+static const TypeInfo core99_machine_info = {
+.name = "mac99-machine",
+.parent= TYPE_MACHINE,
+.class_init= core99_machine_class_init,
};
-static void core99_machine_init(void)
+static void mac_machine_register_types(void)
{
-qemu_register_machine(&core99_machine);
+type_register_static(&core99_machine_info);
}
-machine_init(core99_machine_init);
+type_init(mac_machine_register_types)
--
2.1.0
Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend.
On 01/05/2015 11:21 AM, Gal Hammer wrote:
The monitor's auto-completion feature stopped working when stdio is used
as an input and qemu was resumed after it was suspended (using ctrl-z).
Thanks Gal, it works now!
Tested-by: Marcel Apfelbaum
Signed-off-by: Gal Hammer
---
qemu-char.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index ef84b53..786df33 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1113,12 +1113,22 @@ static int old_fd0_flags;
static bool stdio_in_use;
static bool stdio_allow_signal;
+static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo);
+
static void term_exit(void)
{
tcsetattr (0, TCSANOW, &oldtty);
fcntl(0, F_SETFL, old_fd0_flags);
}
+static void term_stdio_handler(int sig)
+{
+if (sig == SIGCONT) {
+/* echo should be off after resume from suspend. */
+qemu_chr_set_echo_stdio(NULL, false);
+}
+}
+
static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
{
struct termios tty;
@@ -1165,6 +1175,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio
*opts)
tcgetattr(0, &oldtty);
qemu_set_nonblock(0);
atexit(term_exit);
+signal(SIGCONT, term_stdio_handler);
chr = qemu_chr_open_fd(0, 1);
chr->chr_close = qemu_chr_close_stdio;
Re: [Qemu-devel] [PATCH 01/10] block/dmg: properly detect the UDIF trailer
On Sat, Dec 27, 2014 at 04:01:35PM +0100, Peter Wu wrote:
> diff --git a/block/dmg.c b/block/dmg.c
> index e455886..df274f9 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -131,6 +131,39 @@ static void update_max_chunk_size(BDRVDMGState *s,
> uint32_t chunk,
> }
> }
>
> +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs)
> +{
> +int64_t length;
> +int64_t offset = 0;
> +uint8_t buffer[515];
> +int i, ret;
> +
> +/* bdrv_getlength returns a multiple of block size (512), rounded up.
> Since
> + * dmg images can have odd sizes, try to look for the "koly" magic which
> + * marks the begin of the UDIF trailer (512 bytes). This magic can be
> found
> + * in the last 511 bytes of the second-last sector or the first 4 bytes
> of
> + * the last sector (search space: 515 bytes) */
> +length = bdrv_getlength(file_bs);
> +if (length < 512) {
> +return length < 0 ? length : -EINVAL;
dmg_open() should pass in Error *errp so a detailed error reporting can
be used:
if (length < 0) {
error_setg_errno(errp, -length, "Failed to get file size while reading UDIF
trailer");
return length;
} else if (length < 512) {
error_set(errp, "dmg file must be at least 512 bytes long");
return -EINVAL;
}
This makes it much easier to pinpoint errors (instead of just -EINVAL)
and also gives the user a hint about the cause.
> +}
> +if (length > 511 + 512) {
> +offset = length - 511 - 512;
> +}
> +length = length < 515 ? length : 515;
> +ret = bdrv_pread(file_bs, offset, buffer, length);
> +if (ret < 4) {
> +return ret < 0 ? ret : -EINVAL;
bdrv_pread() does not return short reads. The return value will either
be length or an error. This could be just:
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to read last sectors in dmg file");
return ret;
}
(The unique error string makes it easy to track down the location where
the error occurs.)
> +}
> +for (i = 0; i < length - 3; i++) {
> +if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
> +buffer[i+2] == 'l' && buffer[i+3] == 'y') {
> +return offset + i;
> +}
> +}
> +return -EINVAL;
error_set(errp, "Not a dmg file, unable to find UDIF footer");
return -EINVAL;
pgpL9AkxcAsL2.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH 02/10] block/dmg: extract mish block decoding functionality
On Sat, Dec 27, 2014 at 04:01:36PM +0100, Peter Wu wrote: > Extract the mish block decoder such that this can be used for other > formats in the future. A new DmgHeaderState struct is introduced to > share state while decoding. > > The code is kept unchanged as much as possible, a "fail" label is added > for example where a simple return would probably do. > > Signed-off-by: Peter Wu > --- > block/dmg.c | 216 > +++- > 1 file changed, 125 insertions(+), 91 deletions(-) Reviewed-by: Stefan Hajnoczi pgpPFeLtI2G74.pgp Description: PGP signature
[Qemu-devel] qemu sources and makefile system
Hi, I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend.
On 5 January 2015 at 09:21, Gal Hammer wrote:
> The monitor's auto-completion feature stopped working when stdio is used
> as an input and qemu was resumed after it was suspended (using ctrl-z).
>
> Signed-off-by: Gal Hammer
> ---
> qemu-char.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index ef84b53..786df33 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1113,12 +1113,22 @@ static int old_fd0_flags;
> static bool stdio_in_use;
> static bool stdio_allow_signal;
>
> +static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo);
> +
> static void term_exit(void)
> {
> tcsetattr (0, TCSANOW, &oldtty);
> fcntl(0, F_SETFL, old_fd0_flags);
> }
>
> +static void term_stdio_handler(int sig)
> +{
> +if (sig == SIGCONT) {
...why do we need this check? We don't register the function
for any other signals...
> +/* echo should be off after resume from suspend. */
> +qemu_chr_set_echo_stdio(NULL, false);
Should echo really be always off, even if the thing using the
char device had set it to on?
> +}
> +}
> +
> static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
> {
> struct termios tty;
> @@ -1165,6 +1175,7 @@ static CharDriverState
> *qemu_chr_open_stdio(ChardevStdio *opts)
> tcgetattr(0, &oldtty);
> qemu_set_nonblock(0);
> atexit(term_exit);
> +signal(SIGCONT, term_stdio_handler);
This should probably be using sigaction() which is what we use
elsewhere for signal handler registration.
-- PMM
[Qemu-devel] [Bug 491345] Re: remote migration fails with message "load of migration failed"
The bug can be closed since I do not have access to VM now to re-verify the issue. ** Changed in: qemu Status: Incomplete => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/491345 Title: remote migration fails with message "load of migration failed" Status in QEMU: Invalid Bug description: Remote migration fails with message "load of migration failed" on the destination after migration Steps to recreate: 1) install qemu frm git clone git://git.savannah.nongnu.org/qemu.git 2)The VM image was shared using nfs 3)qemu cmdline used: Source: /usr/local/bin/qemu-system-x86_64 -name 'vm1' -drive file=win2k3sp2-32.qcow2 -m 6144 --enable-kvm-usbdevice tablet -vnc :0 -monitor stdio Destination: /usr/local/bin/qemu-system-x86_64 -name 'vm1' -drive file=win2k3sp2-32.qcow2 -m 6144 --enable-kvm-usbdevice tablet -vnc :0 -monitor stdio --incoming tcp:0: 5)migrate tcp:destination: uname -a Linux 2.6.30.9-96.fc11.x86_64 #1 SMP Wed Nov 4 00:02:04 EST 2009 x86_64 x86_64 x86_64 GNU/Linux Distro: fedora 11 Thx yogi To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/491345/+subscriptions
[Qemu-devel] Sources and makefile system
I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the src>Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
[Qemu-devel] qemu sources and makefile system
Hi, I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
[Qemu-devel] qemu sources and makefile system
Hi, I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
[Qemu-devel] Custom machine configuration for QEMU
Hi, I am posting here after searching in vain for quite sometime. Hopefully I've reached the right forum, and hope that I'll get an answer to my questions. We have a board that has a multicore processor - an ARM9 core and a R4 core - and a few peripherals around. We run embedded linux on the ARM9 core and a realtime OS on the R4 core with a custom protocol providing for the communication between then. Can I emulate this setup using QEMU? Specifically, does QEMU support this asymmetric processor configuration? How do I create a machine config corresponding to this board? What customizations will I have to do to QEMU? As a follow up, are there any best practices for emulating the peripheral hardware? Appreciate your help on this! Thanks, Radha.
[Qemu-devel] Sources and makefile system
I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the src>Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
[Qemu-devel] qemu sources and makefile system
Hi, I'm new to qemu-devel and I'm trying to add a ".c" source to qemu. To be more specific, I'm trying to add a file into /hw/virtio/. I've added "common-obj-y += virtio-src.o" to the src>Makefile.objs in that folder and when I'm compiling qemu it seems to compile the sources, but I don't know if they are added to the qemu binary. Is there anything else left to do in order for qemu build system to include my source file? Cata
Re: [Qemu-devel] Custom machine configuration for QEMU
On 6 January 2015 at 11:26, Radha Krishna Srimanthula wrote: > We have a board that has a multicore processor - an ARM9 core and a R4 core > - and a few peripherals around. We run embedded linux on the ARM9 core and a > realtime OS on the R4 core with a custom protocol providing for the > communication between then. > > Can I emulate this setup using QEMU? Specifically, does QEMU support this > asymmetric processor configuration? The short answer is "no". At the moment we can only handle emulating a single symmetric CPU cluster, I'm afraid. -- PMM
Re: [Qemu-devel] [PATCH v4] block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD
On Fri, Jan 02, 2015 at 04:44:38PM -0500, Programmingkid wrote: > Removes redundant ret variable and renames sectorSize variable to meet QEMU > coding standards. This is a changelog item for v4 of this patch. Changelogs should go below the '---' line so they are not merged into git history. The rationale is that when a patch is merged into git, the changelog describing patch revisions that were posted on the mailing list is not relevant (we only see the final patch in git, not the revisions from the mailing list). Patches usually look like this: Subject: block/raw-posix: brief summary A longer description of the problem, maybe a command-line to reproduce a bug, and some rationale for this code change. Signed-off-by: Me --- v2: * Fix int -> size_t for memory lengths [Requested by Bob] The changelog at the bottom is useful to code reviewers but won't get merged in the git history. Anyway, thanks for this patch. I have dropped this changelog line and merged it! > > Signed-off-by: John Arbuckle > > --- > block/raw-posix.c | 18 +- > configure |2 +- > 2 files changed, 18 insertions(+), 2 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpCYc8tKWFLM.pgp Description: PGP signature
Re: [Qemu-devel] Custom machine configuration for QEMU
On Tue Jan 06 2015 at 19:30:02 Peter Maydell wrote: > On 6 January 2015 at 11:26, Radha Krishna Srimanthula > wrote: > > We have a board that has a multicore processor - an ARM9 core and a R4 > core > > - and a few peripherals around. We run embedded linux on the ARM9 core > and a > > realtime OS on the R4 core with a custom protocol providing for the > > communication between then. > > > > Can I emulate this setup using QEMU? Specifically, does QEMU support this > > asymmetric processor configuration? > > The short answer is "no". At the moment we can only handle emulating > a single symmetric CPU cluster, I'm afraid. > > -- PMM > Thanks Peter! If it was a symmetric CPU cluster, how do I go about creating a machine configuration? Any pointers please? Regards, Radha.
Re: [Qemu-devel] Custom machine configuration for QEMU
On 6 January 2015 at 14:20, Radha Krishna Srimanthula wrote: > If it was a symmetric CPU cluster, how do I go about creating a machine > configuration? Any pointers please? Look at an existing board model (preferably one that has been recently added or maintained) and see what it does... -- PMM
Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend.
On 06/01/2015 15:49, Peter Maydell wrote:
On 5 January 2015 at 09:21, Gal Hammer wrote:
The monitor's auto-completion feature stopped working when stdio is used
as an input and qemu was resumed after it was suspended (using ctrl-z).
Signed-off-by: Gal Hammer
---
qemu-char.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index ef84b53..786df33 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1113,12 +1113,22 @@ static int old_fd0_flags;
static bool stdio_in_use;
static bool stdio_allow_signal;
+static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo);
+
static void term_exit(void)
{
tcsetattr (0, TCSANOW, &oldtty);
fcntl(0, F_SETFL, old_fd0_flags);
}
+static void term_stdio_handler(int sig)
+{
+if (sig == SIGCONT) {
...why do we need this check? We don't register the function
for any other signals...
It's a caution and not a must. It can be removed if you think it is
redundant.
+/* echo should be off after resume from suspend. */
+qemu_chr_set_echo_stdio(NULL, false);
Should echo really be always off, even if the thing using the
char device had set it to on?
That's what the function qemu_chr_open_stdio() do, always set the stdio
char device echo to off. I didn't change the current behavior I just
restore it.
As I understood from my tests, the auto-complete feature doesn't work if
the echo is enabled because pressing the tab key prints a tab char.
+}
+}
+
static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
{
struct termios tty;
@@ -1165,6 +1175,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio
*opts)
tcgetattr(0, &oldtty);
qemu_set_nonblock(0);
atexit(term_exit);
+signal(SIGCONT, term_stdio_handler);
This should probably be using sigaction() which is what we use
elsewhere for signal handler registration.
signal() is used in the code. Although I can switch to sigaction() and
earn few more LOC ;-).
-- PMM
Gal.
Re: [Qemu-devel] [PATCH] char: disable stdio echo on resume from suspend.
On 6 January 2015 at 14:30, Gal Hammer wrote:
> On 06/01/2015 15:49, Peter Maydell wrote:
>>
>> On 5 January 2015 at 09:21, Gal Hammer wrote:
>>>
>>> The monitor's auto-completion feature stopped working when stdio is used
>>> as an input and qemu was resumed after it was suspended (using ctrl-z).
>>> +/* echo should be off after resume from suspend. */
>>> +qemu_chr_set_echo_stdio(NULL, false);
>>
>>
>> Should echo really be always off, even if the thing using the
>> char device had set it to on?
>
>
> That's what the function qemu_chr_open_stdio() do, always set the stdio char
> device echo to off. I didn't change the current behavior I just restore it.
But qemu_chr_open_stdio also registers this function as the
chr_set_echo pointer in the CharDriverState, so if the
user of the CharDriverState calls qemu_chr_fe_set_echo(chr, true)
then we'll set the echo to on. And then if we ^Z and resume, your
patch will end up setting the echo to off, which seems wrong.
> As I understood from my tests, the auto-complete feature doesn't work if the
> echo is enabled because pressing the tab key prints a tab char.
Right, if the echo was disabled we want to make sure it is disabled.
But if the echo wasn't disabled we don't want to disable it.
>>> +}
>>> +}
>>> +
>>> static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
>>> {
>>> struct termios tty;
>>> @@ -1165,6 +1175,7 @@ static CharDriverState
>>> *qemu_chr_open_stdio(ChardevStdio *opts)
>>> tcgetattr(0, &oldtty);
>>> qemu_set_nonblock(0);
>>> atexit(term_exit);
>>> +signal(SIGCONT, term_stdio_handler);
>>
>>
>> This should probably be using sigaction() which is what we use
>> elsewhere for signal handler registration.
>
>
> signal() is used in the code.
Only when we're setting a signal to SIG_IGN, I think. signal()'s
semantics aren't portable, which is why we use sigaction().
[See the Linux signal(2) manpage for some discussion of this.]
thanks
-- PMM
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On Jan 6, 2015, at 5:04 AM, Peter Maydell wrote: > On 6 January 2015 at 09:47, Peter Maydell wrote: >> Yes, but it's basically making the user manually toggle a >> setting which we should be getting right ourselves. We >> should find out what QEMU's actually not doing correctly >> and fix that. > > First step to find out what's happening: which of the > following combinations give the messed up colours? > > * OSX host, Linux guest [no, at least for me] > * OSX host, OSX guest [yes] > * Linux host, Linux guest [no] > * Linux host, OSX guest ??? Linux host, OSX guest [no] . Mac OS X is the only operating system I know that has this problem. I am betting this is a bug with Mac OS X, not QEMU. > > Secondly, are we talking about specifically x86 OSX host, > or both x86 and PPC OSX host, or just PPC OSX host? > And is the OSX guest x86, ppc or both? I am pretty sure only PowerPC and x86 OS X host have this problem. The OS X guest is PowerPC. I have not tried the x86 version yet. http://virtuallyfun.superglobalmegacorp.com/?p=267 This post and picture suggest that Mac OS X x86 as a guest does not have this problem.
Re: [Qemu-devel] [Xen-devel] [v3 1/5] Qemu-Xen-vTPM: Support for Xen stubdom vTPM command line options
> -Original Message- > From: xen-devel-boun...@lists.xen.org > [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Eric Blake > Sent: Tuesday, January 06, 2015 12:07 AM > To: Xu, Quan; qemu-devel@nongnu.org > Cc: lcapitul...@redhat.com; arm...@redhat.com; xen-de...@lists.xen.org > Subject: Re: [Xen-devel] [v3 1/5] Qemu-Xen-vTPM: Support for Xen stubdom > vTPM command line options > > On 12/30/2014 04:02 PM, Quan Xu wrote: > > Signed-off-by: Quan Xu > > This message was missing an In-Reply-To header tying it to the 0/5 cover > letter, making it show up as an independent thread. Please see if you can fix > that for the next submission. Eric, Very sorry for this, and I will ask some opensource veteran to help me. > > > --- > > configure| 14 ++ > > hmp.c| 7 +++ > > qapi-schema.json | 19 --- qemu-options.hx | 13 > > +++-- > > tpm.c| 7 ++- > > 5 files changed, 54 insertions(+), 6 deletions(-) > > > > > +++ b/qapi-schema.json > > @@ -2854,9 +2854,10 @@ > > # > > # @passthrough: TPM passthrough type > > # > > -# Since: 1.5 > > +# @xenstubdoms: TPM xenstubdoms type (since 2.3)## Since 1.5 > > Missing newlines. Will add newlines in next v4. Thanks Quan > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v4 0/7] tests: Add check-block to "make check"
On Sun, Jan 04, 2015 at 09:53:45AM +0800, Fam Zheng wrote: > qemu-iotests contains useful tests that have a nice coverage of block layer > code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make > check" is good for developers' self-testing. > > v4: 06: Use CONFIG_LINUX instead of custom "uname" in tests/Makefile. (Peter) > > v2: Take care of other platforms, basically by keeping them unchanged, and > only > add "make check-block" to "make check" on Linux. (Peter) > > > Fam Zheng (7): > .gitignore: Ignore generated "common.env" > qemu-iotests: Remove 091 from quick group > qemu-iotests: Replace "/bin/true" with "true" > qemu-iotests: Add "_supported_os Linux" to 058 > qemu-iotests: Speed up make check-block > tests/Makefile: Add check-block to make check on Linux > qemu-iotests: Add supported os parameter for python tests > > .gitignore | 1 + > tests/Makefile | 3 +++ > tests/qemu-iotests-quick.sh | 2 +- > tests/qemu-iotests/058 | 1 + > tests/qemu-iotests/check | 1 + > tests/qemu-iotests/common.config | 2 +- > tests/qemu-iotests/common.filter | 2 +- > tests/qemu-iotests/common.rc | 2 +- > tests/qemu-iotests/group | 2 +- > tests/qemu-iotests/iotests.py| 5 - > 10 files changed, 15 insertions(+), 6 deletions(-) > > -- > 1.9.3 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpE9G8VXT4uu.pgp Description: PGP signature
Re: [Qemu-devel] [RFC][PATCH] qemu_opt_get_bool_helper: back finding desc by name just if !opt->desc
On Tue, Jan 06, 2015 at 10:39:13AM +0800, Chen, Tiejun wrote:
> On 2015/1/6 9:21, Chen, Tiejun wrote:
> >On 2015/1/6 1:13, Eric Blake wrote:
> >>On 01/04/2015 10:35 PM, Tiejun Chen wrote:
> >>>After one commit 49d2e648e808, "machine: remove qemu_machine_opts
> >>>global list", is introduced, QEMU doesn't keep a global list of
> >>>options but set desc lately. Then we can see the following,
> >>>
> >>>$ x86_64-softmmu/qemu-system-x86_64 -usb
> >>>qemu-system-x86_64: util/qemu-option.c:387: qemu_opt_get_bool_helper: \
> >>> Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
> >>>Aborted (core dumped)
> >>>
> >>>So inside qemu_opt_get_bool_helper, we need to call find_desc_by_name()
> >>>to work parse_option_bool() out just in case of !opt->desc.
> >>>
> >>>Signed-off-by: Tiejun Chen
> >>>---
> >>> util/qemu-option.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/util/qemu-option.c b/util/qemu-option.c
> >>>index a708241..7cb3601 100644
> >>>--- a/util/qemu-option.c
> >>>+++ b/util/qemu-option.c
> >>>@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts
> >>>*opts, const char *name,
> >>> }
> >>>
> >>> opt = qemu_opt_find(opts, name);
> >>>-if (opt == NULL) {
> >>>+if ((opt == NULL) || !opt->desc) {
> >>
> >>Over-parenthesized, and looks like you also introduced a spurious space.
> >> Simpler to just have:
> >>
> >>if (!opt || !opt->desc) {
> >
> >Looks good.
> >
> >>
> >>Also, there are other threads about the same topic.
> >>https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg00130.html
> >
> >Yeah, I already notice that.
> >
>
> Then I realize I need to extend this to all qemu_opt_get_*.
Marcel is working on a fix.
I think hacking qemu_opt_get_* is not a clean solution. The intention
was to move away from QemuOpts to QOM properties. The patch that
removed -machine QemuOptsList descriptions was buggy, but the proper fix
is to make that approach work instead of hacking qemu_opt_get_*.
Stefan
pgpNYDLvTVRo7.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On Tue, Jan 06, 2015 at 10:37:25AM +0800, Chen, Tiejun wrote:
> On 2015/1/5 20:14, Marcel Apfelbaum wrote:
> >On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
> >>On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka
> >>wrote:
> >>>On 2015-01-05 12:22, Stefan Hajnoczi wrote:
> Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
> qemu_machine_opts global list") removed option descriptions from the
> -machine QemuOptsList to avoid repeating MachineState's QOM properties.
>
> This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
> be used on QemuOptsList without option descriptions since QemuOpts
> doesn't know the type and therefore left an unparsed string value.
>
> This patch avoids calling qemu_opt_get_bool() to fix the assertion
> failure:
>
> $ qemu-system-x86_64 -usb
> qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type
> == QEMU_OPT_BOOL' failed.
>
> Test the presence of -usb using qemu_opt_find() but use the
> MachineState->usb field instead of qemu_opt_get_bool().
>
> Cc: Marcel Apfelbaum
> Cc: Tiejun Chen
> Signed-off-by: Stefan Hajnoczi
> ---
> vl.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index bea9656..6e8889c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
> *opaque)
>
> bool usb_enabled(bool default_usb)
> {
> -return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
> - has_defaults && default_usb);
> +if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
> +return current_machine->usb;
> +} else {
> +return has_defaults && default_usb;
> +}
> }
>
> #ifndef _WIN32
>
> >>>
> >>>That still leaves the other boolean machine options broken. A generic
> >>>fix would be good. Or revert the original commit until we have one.
> >>
> >>I think we should revert the original commit.
> >>
> >>qemu_option_get_*() callers need to be converted to query MachineState
> >>fields instead of using QemuOpts functions.
>
> Can this work out currently?
>
> ---
> util/qemu-option.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index a708241..933b885 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char
> *name)
> }
>
> opt = qemu_opt_find(opts, name);
> -if (!opt) {
> +if (!opt || !opt->desc) {
No, this is incorrect because now it just assigns the default value and
ignores the value from the command-line!
Stefan
pgpfvnLJwCm1m.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v4] block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD
On Jan 6, 2015, at 9:02 AM, Stefan Hajnoczi wrote: > On Fri, Jan 02, 2015 at 04:44:38PM -0500, Programmingkid wrote: >> Removes redundant ret variable and renames sectorSize variable to meet QEMU >> coding standards. > > This is a changelog item for v4 of this patch. Changelogs should go > below the '---' line so they are not merged into git history. > > The rationale is that when a patch is merged into git, the changelog > describing patch revisions that were posted on the mailing list is not > relevant (we only see the final patch in git, not the revisions from the > mailing list). > > Patches usually look like this: > > Subject: block/raw-posix: brief summary > > A longer description of the problem, maybe a command-line to reproduce a > bug, and some rationale for this code change. > > Signed-off-by: Me > --- > v2: > * Fix int -> size_t for memory lengths [Requested by Bob] > > The changelog at the bottom is useful to code reviewers but won't get > merged in the git history. > > Anyway, thanks for this patch. I have dropped this changelog line and > merged it! > >> >> Signed-off-by: John Arbuckle >> >> --- >> block/raw-posix.c | 18 +- >> configure |2 +- >> 2 files changed, 18 insertions(+), 2 deletions(-) > > Thanks, applied to my block tree: > https://github.com/stefanha/qemu/commits/block > > Stefan Thank you very much for accepting my patch.
Re: [Qemu-devel] [PATCH v2 07/10] rocker: add new rocker switch device
On Mon, Jan 05, 2015 at 06:24:58PM -0800, sfel...@gmail.com wrote: > From: Scott Feldman > > Rocker is a simulated ethernet switch device. The device supports up to 62 > front-panel ports and supports L2 switching and L3 routing functions, as well > as L2/L3/L4 ACLs. The device presents a single PCI device for each switch, > with a memory-mapped register space for device driver access. > > Rocker device is invoked with -device, for example a 4-port switch: > > -device rocker,name=sw1,len-ports=4,ports[0]=dev0,ports[1]=dev1, \ > ports[2]=dev2,ports[3]=dev3 > > Each port is a netdev and can be paired with using -netdev id=. This design looks good, it fits the QEMU network subsystem. Please follow QEMU coding style, for example, using typedefs for structs instead of "struct tag". Details are in ./HACKING, ./CODING_STYLE, and you can scan patches with QEMU's scripts/checkpatch.pl. pgp4XKJbQd8YV.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v2 08/10] qmp: add rocker device support
On Mon, Jan 05, 2015 at 06:24:59PM -0800, sfel...@gmail.com wrote: > From: Scott Feldman > > Add QMP/HMP support for rocker devices. This is mostly for debugging purposes > to see inside the device's tables and port configurations. Some examples: > > (qemu) rocker sw1 > name: sw1 > id: 0x013512005452 > ports: 4 The convention is for HMP commands that show information to be "info" sub-commands. So this example would be "info rocker sw1". See monitor.c:info_cmds[]. The convention for QMP is to call the command "query-rocker". > > (qemu) rocker-ports sw1 > ena/speed/ auto > port linkduplex neg? > sw1.1 up 10G FD No > sw1.2 up 10G FD No > sw1.3 !ena 10G FD No > sw1.4 !ena 10G FD No HMP: info rocker-ports sw1 QMP: query-rocker-ports and so on. > +## > +# @RockerOfDpaFlow: > +# > +# Rocker switch OF-DPA flow > +# > +# @cookie: flow unique cookie ID > +# > +# @hits: count of matches (hits) on flow > +# > +# @key: flow key > +# > +# @mask: flow mask > +# > +# @action: flow action > +## For versioning, the schema should mention which QEMU release a command was added in: # Since: 2.3 That way QAPI consumers can plan accordingly and be aware when older QEMU binaries may not implement a command. pgpA7Zu5NkQ7c.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v2 0/7] coroutine: optimizations
On Tue, Dec 02, 2014 at 12:05:43PM +0100, Paolo Bonzini wrote: > As discussed in the other thread, this brings speedups from > dropping the coroutine mutex (which serializes multiple iothreads, > too) and using ELF thread-local storage. > > The speedup in perf/cost is about 50% (190->125). Windows port tested > with tests/test-coroutine.exe under Wine. > > Paolo > > v1->v2: include the noinline attribute [many...] > do not mention SwitchToFiber [Kevin] > rename run_main_iothread_exit -> run_main_thread_exit > leave personal opinions out of commit messages :) [Kevin] > mention gain from patch 7 [Peter] > change "alloc_pool_size +=" to "alloc_pool_size =" [Peter] > > Paolo Bonzini (7): > coroutine-ucontext: use __thread > qemu-thread: add per-thread atexit functions > test-coroutine: avoid overflow on 32-bit systems > QSLIST: add lock-free operations > coroutine: rewrite pool to avoid mutex > coroutine: drop qemu_coroutine_adjust_pool_size > coroutine: try harder not to delete coroutines > > block/block-backend.c | 4 -- > coroutine-ucontext.c | 64 +++- > include/block/coroutine.h | 10 - > include/qemu/queue.h | 15 ++- > include/qemu/thread.h | 4 ++ > qemu-coroutine.c | 104 > ++ > tests/test-coroutine.c| 2 +- > util/qemu-thread-posix.c | 37 + > util/qemu-thread-win32.c | 48 - > 9 files changed, 157 insertions(+), 131 deletions(-) > > -- > 2.1.0 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpMYgA9L2dDt.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 1/5] QJSON: Add JSON writer
On 12/26/2014 07:42 AM, Alexander Graf wrote:
> To support programmatic JSON assembly while keeping the code that generates it
> readable, this patch introduces a simple JSON writer. It emits JSON serially
> into a buffer in memory.
>
> The nice thing about this writer is its simplicity and low memory overhead.
> Unlike the QMP JSON writer, this one does not need to spawn QObjects for every
> element it wants to represent.
>
> This is a prerequisite for the migration stream format description generator.
>
> Signed-off-by: Alexander Graf
> ---
> Makefile.objs | 1 +
> include/qjson.h | 28 +
> qjson.c | 97
> +
> 3 files changed, 126 insertions(+)
> create mode 100644 include/qjson.h
> create mode 100644 qjson.c
> +struct QJSON {
> +QString *str;
> +bool omit_comma;
> +unsigned long self_size_offset;
Would size_t be smarter for this field?
> +}
> +
> +const char *qjson_get_str(QJSON *json)
> +{
> +return qstring_get_str(json->str);
> +}
> +
> +QJSON *qjson_new(void)
> +{
> +QJSON *json = g_new(QJSON, 1);
> +json->str = qstring_from_str("{ ");
> +json->omit_comma = true;
> +return json;
If I'm not mistaken, this creates an open-ended object, as in "{ ...".
Should qjson_get_str add the closing }?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: limited request size in write zeroes unsupported path
On Mon, Jan 05, 2015 at 12:29:49PM +0100, Peter Lieven wrote: > If bs->bl.max_write_zeroes is large and we end up in the unsupported > path we might allocate a lot of memory for the iovector and/or even > generate an oversized requests. > > Fix this by limiting the request by the minimum of the reported > maximum transfer size or 16MB (32768 sectors). > > Reported-by: Denis V. Lunev > Signed-off-by: Peter Lieven > --- > block.c |5 - > 1 file changed, 4 insertions(+), 1 deletion(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpSZF8UvAkHk.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] block: limited request size in write zeroes unsupported path
On Mon, Jan 05, 2015 at 03:34:07PM +0300, Denis V. Lunev wrote: > Though pls consider my patch v3, it avoids allocation of 16 Mb here and > uses only 1 Mb of memory. Once your patch has Reviewed-by: it will show up on my radar for merge. If you and Peter need a 2nd opinion in your discussions about the fallocate series, I can look at the series in more detail myself. Just let me know. Stefan pgpvdK7BKiKJk.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 2/5] QJSON: Add JSON writer
On 12/26/2014 07:42 AM, Alexander Graf wrote:
> To support programmatic JSON assembly while keeping the code that generates it
> readable, this patch introduces a simple JSON writer. It emits JSON serially
> into a buffer in memory.
>
> The nice thing about this writer is its simplicity and low memory overhead.
> Unlike the QMP JSON writer, this one does not need to spawn QObjects for every
> element it wants to represent.
>
> This is a prerequisite for the migration stream format description generator.
>
> Signed-off-by: Alexander Graf
>
> ---
>
> v2 -> v3:
>
> - QOMify the QJSON object, makes for easier destruction
> ---
> include/qjson.h | 3 ++-
> qjson.c | 39 ---
> 2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/include/qjson.h b/include/qjson.h
> index 8f8c145..7c54fdf 100644
> --- a/include/qjson.h
> +++ b/include/qjson.h
> @@ -4,7 +4,7 @@
> * Copyright Alexander Graf
> *
> * Authors:
> - * Alexander Graf + * Alexander Graf
Umm, this should be squashed into 1/5.
> @@ -85,9 +88,7 @@ const char *qjson_get_str(QJSON *json)
>
> QJSON *qjson_new(void)
> {
> -QJSON *json = g_new(QJSON, 1);
> -json->str = qstring_from_str("{ ");
> -json->omit_comma = true;
> +QJSON *json = (QJSON *)object_new(TYPE_QJSON);
> return json;
This undoes the dangling object that I complained about on patch 1;
maybe there's some more squashing to do?...
> +static void qjson_initfn(Object *obj)
> +{
> +QJSON *json = (QJSON *)object_dynamic_cast(obj, TYPE_QJSON);
> +assert(json);
> +
> +json->str = qstring_from_str("{ ");
> +json->omit_comma = true;
...or is it still an incomplete object, just now in a different location?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 3/5] qemu-file: Add fast ftell code path
On 12/26/2014 07:42 AM, Alexander Graf wrote: > For ftell we flush the output buffer to ensure that we don't have anything > lingering in our internal buffers. This is a very safe thing to do. > > However, with the dynamic size measurement that the dynamic vmstate > description will bring this would turn out quite slow. > > Instead, we can fast path this specific measurement and just take the > internal buffers into account when telling the kernel our position. > > I'm sure I overlooked some corner cases where this doesn't work, so > instead of tuning the safe, existing version, this patch adds a fast > variant of ftell that gets used by the dynamic vmstate description code > which isn't critical when it fails. > > Signed-off-by: Alexander Graf > > --- > > v2 -> v3: > > - improve ftell_fast, now works with bdrv too > --- > include/migration/qemu-file.h | 1 + > migration/qemu-file.c | 16 > 2 files changed, 17 insertions(+) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 4/5] migration: Append JSON description of migration stream
On 12/26/2014 07:42 AM, Alexander Graf wrote:
> One of the annoyances of the current migration format is the fact that
> it's not self-describing. In fact, it's not properly describing at all.
> Some code randomly scattered throughout QEMU elaborates roughly how to
> read and write a stream of bytes.
>
> We discussed an idea during KVM Forum 2013 to add a JSON description of
> the migration protocol itself to the migration stream. This patch
> adds a section after the VM_END migration end marker that contains
> description data on what the device sections of the stream are composed of.
>
> With an additional external program this allows us to decipher the
> contents of any migration stream and hopefully make migration bugs easier
> to track down.
Hmm. The new format IS self-describing, but ONLY because JSON is
technically possible to parse in reverse. That is, you cannot reliably
look for end-of-stream marker from the beginning of the stream and
reliably get to the JSON description at the end every single time
(because if you don't already know how to interpret the stream, how can
you find end-of-stream), but you CAN reliably look to see if the stream
ends in a JSON object, and then scan backwards for the matching { that
opens the object.
I'm still wondering if you ought to throw in any additional tricks to
make finding the start of the JSON object much easier, such as a
subsection near the beginning of the migration stream, and/or a final
offset description at the tail of the file that says where the JSON
object starts (no additional help for a pipeline, which still has to
store things in memory to loc. Even doing something like:
stream EOS [ { description... }, offset-of-description ]
and using a JSON array rather than a JSON object as the description
would make it much faster to find the start of the JSON object without
having to scan backwards for matching { (although sticking the offset at
the tail doesn't help the issue that when receiving the migration stream
over a pipeline, you still have to reserve memory for the entire stream
in order to find that offset).
>
> Signed-off-by: Alexander Graf
>
> ---
>
> v2 -> v3:
>
> - Dont compress objects with subsections
> - Destroy QJSON object
> ---
> hw/pci/pci.c | 2 +-
> hw/scsi/spapr_vscsi.c | 2 +-
> hw/virtio/virtio.c| 2 +-
> include/migration/migration.h | 1 +
> include/migration/vmstate.h | 3 +-
> migration/vmstate.c | 186
> --
> savevm.c | 54 ++--
> tests/test-vmstate.c | 6 +-
> 8 files changed, 237 insertions(+), 19 deletions(-)
>
At this point, I haven't actually reviewed the patch (I'm more
interested in seeing the JSON it generates), but want to make sure we're
getting an optimal design.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 4/4] arm: enable Bochs PCI VGA
Some ARM platforms can successfully map PCI devices into the guest, so it only makes sense to also add support for the Bochs virtual VGA adapter on those. Signed-off-by: Alexander Graf --- default-configs/arm-softmmu.mak | 1 + 1 file changed, 1 insertion(+) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 7671ee2..1581c85 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -83,6 +83,7 @@ CONFIG_VERSATILE_PCI=y CONFIG_VERSATILE_I2C=y CONFIG_PCI_GENERIC=y +CONFIG_VGA_PCI=y CONFIG_SDHCI=y CONFIG_INTEGRATOR_DEBUG=y -- 1.7.12.4
[Qemu-devel] [PATCH 3/4] arm: Add PCIe host bridge in virt machine
Now that we have a working "generic" PCIe host bridge driver, we can plug
it into ARMs virt machine to always have PCIe available to normal ARM VMs.
I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
into an AArch64 VM with this and they all lived happily ever after.
Signed-off-by: Alexander Graf
---
Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
systems. If you want to use it with AArch64 guests, please apply the following
patch or wait until upstream cleaned up the code properly:
http://csgraf.de/agraf/pci/pci-3.19.patch
---
default-configs/arm-softmmu.mak | 2 +
hw/arm/virt.c | 83 ++---
2 files changed, 80 insertions(+), 5 deletions(-)
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index f3513fa..7671ee2 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
CONFIG_VERSATILE_PCI=y
CONFIG_VERSATILE_I2C=y
+CONFIG_PCI_GENERIC=y
+
CONFIG_SDHCI=y
CONFIG_INTEGRATOR_DEBUG=y
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2353440..b7635ac 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,6 +42,7 @@
#include "exec/address-spaces.h"
#include "qemu/bitops.h"
#include "qemu/error-report.h"
+#include "hw/pci-host/gpex.h"
#define NUM_VIRTIO_TRANSPORTS 32
@@ -69,6 +70,7 @@ enum {
VIRT_MMIO,
VIRT_RTC,
VIRT_FW_CFG,
+VIRT_PCIE,
};
typedef struct MemMapEntry {
@@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
[VIRT_FW_CFG] = { 0x0902, 0x000a },
[VIRT_MMIO] = { 0x0a00, 0x0200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
-/* 0x1000 .. 0x4000 reserved for PCI */
+[VIRT_PCIE] = { 0x1000, 0x3000 },
[VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
};
static const int a15irqmap[] = {
[VIRT_UART] = 1,
[VIRT_RTC] = 2,
+[VIRT_PCIE] = 3,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
};
@@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
}
}
-static void fdt_add_gic_node(const VirtBoardInfo *vbi)
+static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
{
uint32_t gic_phandle;
@@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
2, vbi->memmap[VIRT_GIC_CPU].base,
2, vbi->memmap[VIRT_GIC_CPU].size);
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
+
+return gic_phandle;
}
-static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
+static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
{
/* We create a standalone GIC v2 */
DeviceState *gicdev;
@@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq
*pic)
pic[i] = qdev_get_gpio_in(gicdev, i);
}
-fdt_add_gic_node(vbi);
+return fdt_add_gic_node(vbi);
}
static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -556,6 +561,71 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
g_free(nodename);
}
+static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
+uint32_t gic_phandle)
+{
+hwaddr base = vbi->memmap[VIRT_PCIE].base;
+hwaddr size = vbi->memmap[VIRT_PCIE].size;
+hwaddr size_ioport = 64 * 1024;
+hwaddr size_ecam = PCIE_MMCFG_SIZE_MIN;
+hwaddr size_mmio = size - size_ecam - size_ioport;
+hwaddr base_mmio = base;
+hwaddr base_ioport = base_mmio + size_mmio;
+hwaddr base_ecam = base_ioport + size_ioport;
+int irq = vbi->irqmap[VIRT_PCIE];
+MemoryRegion *mmio_alias;
+MemoryRegion *mmio_reg;
+DeviceState *dev;
+char *nodename;
+
+dev = qdev_create(NULL, TYPE_GPEX_HOST);
+
+qdev_prop_set_uint64(dev, "mmio_window_size", size_mmio);
+qdev_init_nofail(dev);
+
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base_ecam);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_ioport);
+
+/* Map the MMIO window at the same spot in bus and cpu layouts */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg, base_mmio, size_mmio);
+memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
+
+nodename = g_strdup_printf("/pcie@%" PRIx64, base);
+qemu_fdt_add_subnode(vbi->fdt, nodename);
+qemu_fdt_setprop_string(vbi->fdt, nodename,
+"compatible", "pci-host-ecam-generic");
+qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
+qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 3);
+qemu_fdt_setprop_cell(vbi->fdt, node
[Qemu-devel] [PATCH 0/4] ARM: Add support for a generic PCI Express host bridge
Linux implements a nice binding to describe a "generic" PCI Express host bridge using only device tree. This patch set adds enough emulation logic to expose the parts that are "generic" as a simple sysbus device and maps it into ARM's virt machine. With this patch set, we can finally spawn PCI devices on ARM VMs. I was able to have a fully DRM enabled virtual machine with VGA, e1000 and XHCI (for keyboard and mouse) up and working. It's only a small step for QEMU, but a big step for ARM VM's usability. Happy new year! Alexander Graf (4): pci: Split pcie_host_mmcfg_map() pci: Add generic PCIe host bridge arm: Add PCIe host bridge in virt machine arm: enable Bochs PCI VGA default-configs/arm-softmmu.mak | 3 + hw/arm/virt.c | 83 +++-- hw/pci-host/Makefile.objs | 1 + hw/pci-host/gpex.c | 156 hw/pci/pcie_host.c | 9 ++- include/hw/pci-host/gpex.h | 56 +++ include/hw/pci/pcie_host.h | 1 + 7 files changed, 302 insertions(+), 7 deletions(-) create mode 100644 hw/pci-host/gpex.c create mode 100644 include/hw/pci-host/gpex.h -- 1.7.12.4
[Qemu-devel] [PATCH 2/4] pci: Add generic PCIe host bridge
With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
can successfully create a workable PCIe host bridge that can be mapped anywhere
and only needs to get described to the OS using whatever means it likes.
This patch implements such a "generic" host bridge. It only supports a single
legacy IRQ line so far. MSIs need to be handled external to the host bridge.
This device is particularly useful for the "pci-host-ecam-generic" driver in
Linux.
Signed-off-by: Alexander Graf
---
hw/pci-host/Makefile.objs | 1 +
hw/pci-host/gpex.c | 156 +
include/hw/pci-host/gpex.h | 56
3 files changed, 213 insertions(+)
create mode 100644 hw/pci-host/gpex.c
create mode 100644 include/hw/pci-host/gpex.h
diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index bb65f9c..45f1f0e 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -15,3 +15,4 @@ common-obj-$(CONFIG_PCI_APB) += apb.o
common-obj-$(CONFIG_FULONG) += bonito.o
common-obj-$(CONFIG_PCI_PIIX) += piix.o
common-obj-$(CONFIG_PCI_Q35) += q35.o
+common-obj-$(CONFIG_PCI_GENERIC) += gpex.o
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
new file mode 100644
index 000..bd62a3c
--- /dev/null
+++ b/hw/pci-host/gpex.c
@@ -0,0 +1,156 @@
+/*
+ * QEMU Generic PCI Express Bridge Emulation
+ *
+ * Copyright (C) 2015 Alexander Graf
+ *
+ * Code loosely based on q35.c.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "hw/hw.h"
+#include "hw/pci-host/gpex.h"
+
+/
+ * GPEX host
+ */
+
+static void gpex_set_irq(void *opaque, int irq_num, int level)
+{
+GPEXHost *s = opaque;
+
+qemu_set_irq(s->irq, level);
+}
+
+static int gpex_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+/* We only support one IRQ line so far */
+return 0;
+}
+
+static void gpex_host_realize(DeviceState *dev, Error **errp)
+{
+PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+GPEXHost *s = GPEX_HOST(dev);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
+
+pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MIN);
+memory_region_init(&s->io_mmio, OBJECT(s), "gpex_mmio",
s->mmio_window_size);
+memory_region_init(&s->io_ioport, OBJECT(s), "gpex_ioport", 64 * 1024);
+
+sysbus_init_mmio(sbd, &pex->mmio);
+sysbus_init_mmio(sbd, &s->io_mmio);
+sysbus_init_mmio(sbd, &s->io_ioport);
+sysbus_init_irq(sbd, &s->irq);
+
+pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq, gpex_map_irq, s,
+ &s->io_mmio, &s->io_ioport, 0, 1, TYPE_PCIE_BUS);
+
+qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
+qdev_init_nofail(DEVICE(&s->gpex_root));
+}
+
+static const char *gpex_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+return ":00";
+}
+
+static Property gpex_root_props[] = {
+DEFINE_PROP_UINT64("mmio_window_size", GPEXHost, mmio_window_size, 1ULL <<
32),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void gpex_host_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
+
+hc->root_bus_path = gpex_host_root_bus_path;
+dc->realize = gpex_host_realize;
+dc->props = gpex_root_props;
+set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+dc->fw_name = "pci";
+}
+
+static void gpex_host_initfn(Object *obj)
+{
+GPEXHost *s = GPEX_HOST(obj);
+
+object_initialize(&s->gpex_root, sizeof(s->gpex_root),
TYPE_GPEX_ROOT_DEVICE);
+object_property_add_child(OBJECT(s), "gpex_root", OBJECT(&s->gpex_root),
NULL);
+qdev_prop_set_uint32(DEVICE(&s->gpex_root), "addr", PCI_DEVFN(0, 0));
+qdev_prop_set_bit(DEVICE(&s->gpex_root), "multifunction", false);
+}
+
+static
Re: [Qemu-devel] [PATCH v3 5/5] Add migration stream analyzation script
On 12/26/2014 07:42 AM, Alexander Graf wrote:
> This patch adds a python tool to the scripts directory that can read
> a dumped migration stream if it contains the JSON description of the
> device states. I constructs a human readable JSON stream out of it.
>
> It's very simple to use:
>
> $ qemu-system-x86_64
> (qemu) migrate "exec:cat > mig"
> $ ./scripts/analyze_migration.py -f mig
>
> Signed-off-by: Alexander Graf
>
> ---
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> new file mode 100755
> index 000..3363172
> --- /dev/null
> +++ b/scripts/analyze-migration.py
> @@ -0,0 +1,592 @@
> +#!/usr/bin/env python
> +#
> +# Migration Stream Analyzer
> +#
> +# Copyright (c) 2013 Alexander Graf
Started in 2013, but you may want to add 2014 and 2015 by the time this
patch is polished.
> +
> +# The VMSD description is at the end of the file, after EOF. Look for
> +# the last NULL byte, then for the beginning brace of JSON.
Hmm, you picked up on an optimization that I missed in my ramblings
about 4/5 :) Since a well-formed JSON description contains no NUL bytes
or control characters, a pipeline read of a migration stream can just
continually look for every '\6' marker byte followed by '{' as the
potential start of an object, and reset that search every time a '\0' or
another '\6' byte is seen; eventually, this will result in just the tail
of the file being seen as the JSON object (assuming that we never add
any other tail metadata - or that all other tail metadata is added as
backwards-compatible extensions to the JSON). It STILL might be worth
the efficiency gain of stashing an offset in the file somewhere (as it
is faster to seek to an offset than it is to scan for particular
patterns), but at least your scan works forwards rather than in reverse.
> +
> +# Find the last NULL byte, then the first brace after that. This
> should
> +# be the beginning of our JSON data.
> +nulpos = data.rfind("\0")
> +jsonpos = data.find("{", nulpos)
Are you sure that there will NEVER be a '{' somewhere between the last
NUL and the end-of-stream marker? Shouldn't you really be searching for
the last QEMU_VM_VMDESCRIPTION ('\6') magic byte, rather than NUL?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 4/4] arm: enable Bochs PCI VGA
On 6 January 2015 at 16:03, Alexander Graf wrote: > Some ARM platforms can successfully map PCI devices into the guest, so it only > makes sense to also add support for the Bochs virtual VGA adapter on those. > > Signed-off-by: Alexander Graf > --- > default-configs/arm-softmmu.mak | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak > index 7671ee2..1581c85 100644 > --- a/default-configs/arm-softmmu.mak > +++ b/default-configs/arm-softmmu.mak > @@ -83,6 +83,7 @@ CONFIG_VERSATILE_PCI=y > CONFIG_VERSATILE_I2C=y > > CONFIG_PCI_GENERIC=y > +CONFIG_VGA_PCI=y Why isn't this just in pci.mak like all the other PCI devices? -- PMM
[Qemu-devel] [PATCH 1/4] pci: Split pcie_host_mmcfg_map()
The mmcfg space is a memory region that allows access to PCI config space
in the PCIe world. To maintain abstraction layers, I would like to expose
the mmcfg space as a sysbus mmio region rather than have it mapped straight
into the system's memory address space though.
So this patch splits the initialization of the mmcfg space from the actual
mapping, allowing us to only have an mmfg memory region without the map.
Signed-off-by: Alexander Graf
---
hw/pci/pcie_host.c | 9 +++--
include/hw/pci/pcie_host.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index 3db038f..dfb4a2b 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -98,8 +98,7 @@ void pcie_host_mmcfg_unmap(PCIExpressHost *e)
}
}
-void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
- uint32_t size)
+void pcie_host_mmcfg_init(PCIExpressHost *e, uint32_t size)
{
assert(!(size & (size - 1))); /* power of 2 */
assert(size >= PCIE_MMCFG_SIZE_MIN);
@@ -107,6 +106,12 @@ void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
e->size = size;
memory_region_init_io(&e->mmio, OBJECT(e), &pcie_mmcfg_ops, e,
"pcie-mmcfg", e->size);
+}
+
+void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
+ uint32_t size)
+{
+pcie_host_mmcfg_init(e, size);
e->base_addr = addr;
memory_region_add_subregion(get_system_memory(), e->base_addr, &e->mmio);
}
diff --git a/include/hw/pci/pcie_host.h b/include/hw/pci/pcie_host.h
index ff44ef6..4d23c80 100644
--- a/include/hw/pci/pcie_host.h
+++ b/include/hw/pci/pcie_host.h
@@ -50,6 +50,7 @@ struct PCIExpressHost {
};
void pcie_host_mmcfg_unmap(PCIExpressHost *e);
+void pcie_host_mmcfg_init(PCIExpressHost *e, uint32_t size);
void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr, uint32_t size);
void pcie_host_mmcfg_update(PCIExpressHost *e,
int enable,
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2 07/10] rocker: add new rocker switch device
On Tue, Jan 6, 2015 at 7:12 AM, Stefan Hajnoczi wrote:
> On Mon, Jan 05, 2015 at 06:24:58PM -0800, sfel...@gmail.com wrote:
>> From: Scott Feldman
>>
>> Rocker is a simulated ethernet switch device. The device supports up to 62
>> front-panel ports and supports L2 switching and L3 routing functions, as well
>> as L2/L3/L4 ACLs. The device presents a single PCI device for each switch,
>> with a memory-mapped register space for device driver access.
>>
>> Rocker device is invoked with -device, for example a 4-port switch:
>>
>> -device rocker,name=sw1,len-ports=4,ports[0]=dev0,ports[1]=dev1, \
>> ports[2]=dev2,ports[3]=dev3
>>
>> Each port is a netdev and can be paired with using -netdev id=.
>
> This design looks good, it fits the QEMU network subsystem.
>
> Please follow QEMU coding style, for example, using typedefs for structs
> instead of "struct tag". Details are in ./HACKING, ./CODING_STYLE, and
> you can scan patches with QEMU's scripts/checkpatch.pl.
The patches are already scripts/checkpatch.pl clean.
And we did follow the HACKING and CODING_STYLE guidelines, with the
exception of typedefs for structs. Did you spot anything else
out-of-compliance?
On typedefs for structs, there are plenty of examples in Qemu of not
following that rule. Perhaps this rule can be enforced by
checkpatch.pl? Personally, I feel that typedef on a struct makes the
code harder to read as it obfuscate the type. For example, typedef
union {...} foo or typedef struct {...} foo or typedef enum {...} foo:
there is no way to tell with foo bar what bar is, at a glance, whereas
union foo bar or struct foo bar or enum foo bar is clear.
We can make the typedef change for v3 if it's a hard requirement for inclusion.
-scott
Re: [Qemu-devel] [RFC][PATCH] qemu_opt_get_bool_helper: back finding desc by name just if !opt->desc
On 01/06/2015 04:56 PM, Stefan Hajnoczi wrote:
On Tue, Jan 06, 2015 at 10:39:13AM +0800, Chen, Tiejun wrote:
On 2015/1/6 9:21, Chen, Tiejun wrote:
On 2015/1/6 1:13, Eric Blake wrote:
On 01/04/2015 10:35 PM, Tiejun Chen wrote:
After one commit 49d2e648e808, "machine: remove qemu_machine_opts
global list", is introduced, QEMU doesn't keep a global list of
options but set desc lately. Then we can see the following,
$ x86_64-softmmu/qemu-system-x86_64 -usb
qemu-system-x86_64: util/qemu-option.c:387: qemu_opt_get_bool_helper: \
Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
Aborted (core dumped)
So inside qemu_opt_get_bool_helper, we need to call find_desc_by_name()
to work parse_option_bool() out just in case of !opt->desc.
Signed-off-by: Tiejun Chen
---
util/qemu-option.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..7cb3601 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts
*opts, const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if ((opt == NULL) || !opt->desc) {
Over-parenthesized, and looks like you also introduced a spurious space.
Simpler to just have:
if (!opt || !opt->desc) {
Looks good.
Also, there are other threads about the same topic.
https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg00130.html
Yeah, I already notice that.
Then I realize I need to extend this to all qemu_opt_get_*.
Marcel is working on a fix.
I just posted a fix on the mailing list.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272607.html
Thanks,
Marcel
I think hacking qemu_opt_get_* is not a clean solution. The intention
was to move away from QemuOpts to QOM properties. The patch that
removed -machine QemuOptsList descriptions was buggy, but the proper fix
is to make that approach work instead of hacking qemu_opt_get_*.
Stefan
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()
On 01/06/2015 11:01 AM, Chen, Tiejun wrote:
On 2015/1/6 14:20, Shannon Zhao wrote:
On 2015/1/6 10:37, Chen, Tiejun wrote:
On 2015/1/5 20:14, Marcel Apfelbaum wrote:
On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote:
On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka
wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
qemu_machine_opts global list") removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type
== QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState->usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum
Cc: Tiejun Chen
Signed-off-by: Stefan Hajnoczi
---
vl.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void
*opaque)
bool usb_enabled(bool default_usb)
{
-return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
- has_defaults && default_usb);
+if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
+return current_machine->usb;
+} else {
+return has_defaults && default_usb;
+}
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
Can this work out currently?
Hi Tiejun,
I apply your following patch and it works. At least it doesn't crash.
Thanks for your test.
Hi,
I just posted a fix on the mailing list.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272607.html
Please check if it works for you,
Thanks,
Marcel
Tiejun
Thanks,
Shannon
---
util/qemu-option.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a708241..933b885 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
return desc->def_value_str;
@@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
}
opt = qemu_opt_find(opts, name);
-if (!opt) {
+if (!opt || !opt->desc) {
desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
str = g_strdup(desc->def_value_str);
@@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const
char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
@@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_number(name, desc->def_value_str, &ret,
&error_abort);
@@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts,
const char *name,
}
opt = qemu_opt_find(opts, name);
-if (opt == NULL) {
+if (!opt || !opt->desc) {
const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
if (desc && desc->def_value_str) {
parse_option_size(name, desc->def_value_str, &ret, &error_abort);
[Qemu-devel] [PATCH v2] target-openrisc: bugfix for dec_sys to decode instructions correctly
Fixed the decoding of "system" instructions (starting with 0x2) in dec_sys() in translate.c. In particular, the l.trap instruction is now correctly decoded, which enables for singlestepping and breakpoints to be set in GDB. Signed-off-by: David R. Morrison --- target-openrisc/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Changes since previous version: * Added 'signed-off-by' line; sorry to forget this before! diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c index 407bd97..d36278f 100644 --- a/target-openrisc/translate.c +++ b/target-openrisc/translate.c @@ -1320,7 +1320,7 @@ static void dec_sys(DisasContext *dc, uint32_t insn) #ifdef OPENRISC_DISAS uint32_t K16; #endif -op0 = extract32(insn, 16, 8); +op0 = extract32(insn, 16, 10); #ifdef OPENRISC_DISAS K16 = extract32(insn, 0, 16); #endif -- 2.2.1
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On Jan 6, 2015, at 11:46 AM, Peter Maydell wrote: > On 6 January 2015 at 16:30, Programmingkid wrote: >> I was doing some searching and thought I should show you this: >> file: vga.c >> >> This indicates that all operations are expected to be in the little endian >> format. > > That controls the endianness to be used when writing words to > the VGA registers, which is not the same as the endianness of > the pixel format (which is what your previous patch was affecting). > > See the comment in vga_common_init(): > >/* > * Set default fb endian based on target, could probably be turned > * into a device attribute set by the machine/platform to remove > * all target endian dependencies from this file. > */ > #ifdef TARGET_WORDS_BIGENDIAN >s->default_endian_fb = true; > #else >s->default_endian_fb = false; > #endif > > What do we sent default_endian_fb to in the configuration that > doesn't give the right colours, and if we set it to the other > setting does it work? > > -- PMM After investigating the TARGET_WORDS_BIGENDIAN code, I noticed that s->default_endian_fb was being set to true. So I undefined the macro and then ran QEMU. The i386 target showed no change in colors. The ppc target still had the same incorrect colors. Sorry, this doesn't fix the problem.
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On 6 January 2015 at 17:19, Programmingkid wrote: > After investigating the TARGET_WORDS_BIGENDIAN code, I noticed > that s->default_endian_fb was being set to true. So I undefined > the macro and then ran QEMU. The i386 target showed no change > in colors. The ppc target still had the same incorrect colors. > Sorry, this doesn't fix the problem. What macro? You don't want to undefine TARGET_WORDS_BIGENDIAN, that will wreak all kinds of havoc. Just test with manually setting default_endian_fb to false. -- PMM
[Qemu-devel] [PATCH] vl.c: fix regression when reading machine type from config file
After 'Machine as QOM' series the machine type input triggers
the creation of the machine class.
If the machine type is set in the configuration file, the machine
class is not updated accordingly and remains the default.
Fixed that by querying the machine options after the configuration
file is loaded.
Cc: qemu-sta...@nongnu.org
Reported-by: William Dauchy
Signed-off-by: Marcel Apfelbaum
---
vl.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/vl.c b/vl.c
index 7786b2f..ecd8c93 100644
--- a/vl.c
+++ b/vl.c
@@ -3659,6 +3659,11 @@ int main(int argc, char **argv, char **envp)
strerror(-ret));
exit(1);
}
+opts = qemu_get_machine_opts();
+optarg = qemu_opt_get(opts, "type");
+if (optarg) {
+machine_class = machine_parse(optarg);
+}
break;
}
case QEMU_OPTION_spice:
--
2.1.0
[Qemu-devel] [PATCH v2 04/12] block/dmg: process a buffer instead of reading ints
As the decoded plist XML is not a pointer in the file,
dmg_read_mish_block must be able to process a buffer instead of a file
pointer. Since the full buffer must be processed, let's change the
return value again to just a success flag.
Signed-off-by: Peter Wu
Reviewed-by: John Snow
---
v2: added R-b
---
block/dmg.c | 60 ++--
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index ed99cf5..4913249 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -100,6 +100,16 @@ static int read_uint32(BlockDriverState *bs, int64_t
offset, uint32_t *result)
return 0;
}
+static inline uint64_t buff_read_uint64(const uint8_t *buffer, int64_t offset)
+{
+return be64_to_cpu(*(uint64_t *)&buffer[offset]);
+}
+
+static inline uint32_t buff_read_uint32(const uint8_t *buffer, int64_t offset)
+{
+return be32_to_cpu(*(uint32_t *)&buffer[offset]);
+}
+
/* Increase max chunk sizes, if necessary. This function is used to calculate
* the buffer sizes needed for compressed/uncompressed chunk I/O.
*/
@@ -184,20 +194,16 @@ typedef struct DmgHeaderState {
uint32_t max_sectors_per_chunk;
} DmgHeaderState;
-static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
- int64_t offset, uint32_t count)
+static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
+ uint8_t *buffer, uint32_t count)
{
-BDRVDMGState *s = bs->opaque;
uint32_t type, i;
int ret;
size_t new_size;
uint32_t chunk_count;
+int64_t offset = 0;
-ret = read_uint32(bs, offset, &type);
-if (ret < 0) {
-goto fail;
-}
-
+type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
if (type != 0x6d697368 || count < 244) {
/* assume success for now */
@@ -216,10 +222,7 @@ static int dmg_read_mish_block(BlockDriverState *bs,
DmgHeaderState *ds,
s->sectorcounts = g_realloc(s->sectorcounts, new_size);
for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
-ret = read_uint32(bs, offset, &s->types[i]);
-if (ret < 0) {
-goto fail;
-}
+s->types[i] = buff_read_uint32(buffer, offset);
offset += 4;
if (s->types[i] != 0x8005 && s->types[i] != 1 &&
s->types[i] != 2) {
@@ -235,17 +238,11 @@ static int dmg_read_mish_block(BlockDriverState *bs,
DmgHeaderState *ds,
}
offset += 4;
-ret = read_uint64(bs, offset, &s->sectors[i]);
-if (ret < 0) {
-goto fail;
-}
+s->sectors[i] = buff_read_uint64(buffer, offset);
s->sectors[i] += ds->last_out_offset;
offset += 8;
-ret = read_uint64(bs, offset, &s->sectorcounts[i]);
-if (ret < 0) {
-goto fail;
-}
+s->sectorcounts[i] = buff_read_uint64(buffer, offset);
offset += 8;
if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
@@ -256,17 +253,11 @@ static int dmg_read_mish_block(BlockDriverState *bs,
DmgHeaderState *ds,
goto fail;
}
-ret = read_uint64(bs, offset, &s->offsets[i]);
-if (ret < 0) {
-goto fail;
-}
+s->offsets[i] = buff_read_uint64(buffer, offset);
s->offsets[i] += ds->last_in_offset;
offset += 8;
-ret = read_uint64(bs, offset, &s->lengths[i]);
-if (ret < 0) {
-goto fail;
-}
+s->lengths[i] = buff_read_uint64(buffer, offset);
offset += 8;
if (s->lengths[i] > DMG_LENGTHS_MAX) {
@@ -290,8 +281,10 @@ fail:
static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
uint64_t info_begin, uint64_t info_length)
{
+BDRVDMGState *s = bs->opaque;
int ret;
uint32_t count, rsrc_data_offset;
+uint8_t *buffer = NULL;
uint64_t info_end;
uint64_t offset;
@@ -332,16 +325,23 @@ static int dmg_read_resource_fork(BlockDriverState *bs,
DmgHeaderState *ds,
}
offset += 4;
-ret = dmg_read_mish_block(bs, ds, offset, count);
+buffer = g_realloc(buffer, count);
+ret = bdrv_pread(bs->file, offset, buffer, count);
+if (ret < 0) {
+goto fail;
+}
+
+ret = dmg_read_mish_block(s, ds, buffer, count);
if (ret < 0) {
goto fail;
}
/* advance offset by size of resource */
offset += count;
}
-return 0;
+ret = 0;
fail:
+g_free(buffer);
return ret;
}
--
2.2.1
[Qemu-devel] [PATCH v2 00/12] block/dmg: (compatibility) fixes and bzip2 support
Hi, This is the second revision of improvements to DMG image file support. See [1] for an overview of the previous patchset. Thanks to John Snow for his efforts in reviewing patches and providing suggestions. The errp suggestion from Stefan Hajnoczi is also incorporated. An overview of changes since v1 (also mentioned in each patch): block/dmg: properly detect the UDIF trailer [+R-b, set errp) block/dmg: extract mish block decoding functionality [+R-b, added comments, expanded commit message, renamed var] block/dmg: extract processing of resource forks [see patch] block/dmg: process a buffer instead of reading ints [+R-b, no changes] block/dmg: validate chunk size to avoid overflow [added offset check] block/dmg: process XML plists [added offset check] block/dmg: set virtual size to a non-zero value [fix commit message] block/dmg: fix sector data offset calculation [use data provided by file] block/dmg: use SectorNumber from BLKX header [new patch] block/dmg: factor out block type check [extracted from bzip patch] block/dmg: support bzip2 block entry types [set/use BZIP2_LIBS] block/dmg: improve zeroes handling [no changes] (the other length checking patch was squashed into the others) Note: in the previous patches I mentioned that dmg2img would result in a shorter file than qemu-img convert. That turns out to be a bug in dmg2img for which a patch is available[2]. A quick test runner is available at https://lekensteyn.nl/files/dmg-tests/. This script depends on the fixed dmg2img program and can then be run as follows: QEMU_IMG=/tmp/qout/qemu-img ./run-dmg-tests.sh dmg-images/*.dmg You will first need some DMG files, I have collected four different public examples with different properties[1]. These can be found in urls.txt at https://lekensteyn.nl/files/dmg-tests/dmg-images/. Kind regards, Peter [1]: https://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg03606.html [2]: https://github.com/Lekensteyn/dmg2img/commit/a1d4861b4b0f2ac5090938233a1156bb130cb3ef Peter Wu (12): block/dmg: properly detect the UDIF trailer block/dmg: extract mish block decoding functionality block/dmg: extract processing of resource forks block/dmg: process a buffer instead of reading ints block/dmg: validate chunk size to avoid overflow block/dmg: process XML plists block/dmg: set virtual size to a non-zero value block/dmg: fix sector data offset calculation block/dmg: use SectorNumber from BLKX header block/dmg: factor out block type check block/dmg: support bzip2 block entry types block/dmg: improve zeroes handling block/Makefile.objs | 1 + block/dmg.c | 503 configure | 31 3 files changed, 418 insertions(+), 117 deletions(-) -- 2.2.1
[Qemu-devel] [PATCH v2 05/12] block/dmg: validate chunk size to avoid overflow
Previously the chunk size was not checked, allowing for a large memory
allocation. This patch checks whether the chunks size is within the
resource fork length, and whether the resource fork is below the
trailer of the dmg file.
Signed-off-by: Peter Wu
---
v2: added resource fork offset check
---
block/dmg.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/dmg.c b/block/dmg.c
index 4913249..5f6976b 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -319,7 +319,7 @@ static int dmg_read_resource_fork(BlockDriverState *bs,
DmgHeaderState *ds,
ret = read_uint32(bs, offset, &count);
if (ret < 0) {
goto fail;
-} else if (count == 0) {
+} else if (count == 0 || count > info_end - offset) {
ret = -EINVAL;
goto fail;
}
@@ -379,6 +379,11 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
if (ret < 0) {
goto fail;
}
+if (rsrc_fork_offset >= offset ||
+rsrc_fork_length > offset - rsrc_fork_offset) {
+ret = -EINVAL;
+goto fail;
+}
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
--
2.2.1
[Qemu-devel] [PATCH v2 01/12] block/dmg: properly detect the UDIF trailer
DMG files have a variable length with a UDIF trailer at the end of a
file. This UDIF trailer is essential as it describes the contents of
the image. At the moment however, the start of this trailer is almost
always incorrect as bdrv_getlength() returns a multiple of the block
size (rounded up). This results in a failure to recognize DMG files,
resulting in Invalid argument (EINVAL) errors.
As there is no API to retrieve the real file size, look for the magic
header in the last two sectors to find the start of this 512-byte UDIF
trailer (the "koly" block).
The resource fork offset ("info_begin") has its offset adjusted as the
initial value of offset does not mean "end of file" anymore, but "begin
of UDIF trailer".
Signed-off-by: Peter Wu
Reviewed-by: John Snow
---
v2: added R-b, set errp as suggested by Stefan Hajnoczi
---
block/dmg.c | 49 +
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index e455886..9183459 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -131,6 +131,48 @@ static void update_max_chunk_size(BDRVDMGState *s,
uint32_t chunk,
}
}
+static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error **errp)
+{
+int64_t length;
+int64_t offset = 0;
+uint8_t buffer[515];
+int i, ret;
+
+/* bdrv_getlength returns a multiple of block size (512), rounded up. Since
+ * dmg images can have odd sizes, try to look for the "koly" magic which
+ * marks the begin of the UDIF trailer (512 bytes). This magic can be found
+ * in the last 511 bytes of the second-last sector or the first 4 bytes of
+ * the last sector (search space: 515 bytes) */
+length = bdrv_getlength(file_bs);
+if (length < 0) {
+error_setg_errno(errp, -length,
+"Failed to get file size while reading UDIF trailer");
+return length;
+} else if (length < 512) {
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+"dmg file must be at least 512 bytes long");
+return -EINVAL;
+}
+if (length > 511 + 512) {
+offset = length - 511 - 512;
+}
+length = length < 515 ? length : 515;
+ret = bdrv_pread(file_bs, offset, buffer, length);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Failed while reading UDIF trailer");
+return ret;
+}
+for (i = 0; i < length - 3; i++) {
+if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
+buffer[i+2] == 'l' && buffer[i+3] == 'y') {
+return offset + i;
+}
+}
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+"Could not locate UDIF trailer in dmg file");
+return -EINVAL;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
@@ -145,15 +187,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
-/* read offset of info blocks */
-offset = bdrv_getlength(bs->file);
+/* locate the UDIF trailer */
+offset = dmg_find_koly_offset(bs->file, errp);
if (offset < 0) {
ret = offset;
goto fail;
}
-offset -= 0x1d8;
-ret = read_uint64(bs, offset, &info_begin);
+ret = read_uint64(bs, offset + 0x28, &info_begin);
if (ret < 0) {
goto fail;
} else if (info_begin == 0) {
--
2.2.1
[Qemu-devel] [PATCH v2 09/12] block/dmg: use SectorNumber from BLKX header
Previously the sector table parsing relied on the previous offset of
the DMG file. Now it uses the sector number from the BLKX header
(see http://newosxbook.com/DMG.html).
The implementation of dmg2img (from vu1tur) does not base the output
sector on the location of the terminator (0x) either so it
should be safe to drop this dependency on the previous state.
(It makes somehow makes sense, a terminator should halt further
processing of a block and is perhaps used to preallocate some space.)
Signed-off-by: Peter Wu
---
v2: initial patch after suggestions from John Snow to read and use
these fields.
---
block/dmg.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 130efac..57922c5 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -189,7 +189,6 @@ typedef struct DmgHeaderState {
/* used internally by dmg_read_mish_block to remember offsets of blocks
* across calls */
uint64_t data_fork_offset;
-uint64_t last_out_offset;
/* exported for dmg_open */
uint32_t max_compressed_size;
uint32_t max_sectors_per_chunk;
@@ -205,6 +204,7 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
int64_t offset = 0;
uint64_t data_offset;
uint64_t in_offset = ds->data_fork_offset;
+uint64_t out_offset;
type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
@@ -213,6 +213,9 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
return 0;
}
+/* chunk offsets are relative to this sector number */
+out_offset = buff_read_uint64(buffer, offset + 8);
+
/* location in data fork for (compressed) blob (in bytes) */
data_offset = buff_read_uint64(buffer, offset + 0x18);
in_offset += data_offset;
@@ -233,10 +236,6 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
offset += 4;
if (s->types[i] != 0x8005 && s->types[i] != 1 &&
s->types[i] != 2) {
-if (s->types[i] == 0x && i > 0) {
-ds->last_out_offset = s->sectors[i - 1] +
- s->sectorcounts[i - 1];
-}
chunk_count--;
i--;
offset += 36;
@@ -245,7 +244,7 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
offset += 4;
s->sectors[i] = buff_read_uint64(buffer, offset);
-s->sectors[i] += ds->last_out_offset;
+s->sectors[i] += out_offset;
offset += 8;
s->sectorcounts[i] = buff_read_uint64(buffer, offset);
@@ -419,7 +418,6 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
/* used by dmg_read_mish_block to keep track of the current I/O position */
ds.data_fork_offset = 0;
-ds.last_out_offset = 0;
ds.max_compressed_size = 1;
ds.max_sectors_per_chunk = 1;
--
2.2.1
[Qemu-devel] [PATCH v2 08/12] block/dmg: fix sector data offset calculation
This patch addresses two issues:
- The data fork offset was not taken into account, resulting in failure
to read an InstallESD.dmg file (5164763151 bytes) which had a
non-zero DataForkOffset field.
- The offset of the previous block ("partition") was unconditionally
added to the current block because older files would start the input
offset of a new block at zero. Newer files (including vlc-2.1.5.dmg,
tuxpaint-0.9.15-macosx.dmg and OS X Yosemite [MAS].dmg) failed in
reads because these files have chunk offsets, relative to the begin
of a data fork.
Now the data offset of the mish is taken into account. While we could
check that the data_offset is within the data fork, let's not do that
here as it would only result in parse failures on invalid files (rather
than gracefully handling such bad files). dmg_read will error out if
the offset is incorrect.
Signed-off-by: Peter Wu
---
v2: use sector and data offset as provided by the BLKX header. This
allows us to drop last_in_offset. The previous heuristics to detect
relative offsets is not needed anymore. Squashed the data fork
offset length check into this patch.
---
block/dmg.c | 26 --
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 57feb1b..130efac 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -188,7 +188,7 @@ static int64_t dmg_find_koly_offset(BlockDriverState
*file_bs, Error **errp)
typedef struct DmgHeaderState {
/* used internally by dmg_read_mish_block to remember offsets of blocks
* across calls */
-uint64_t last_in_offset;
+uint64_t data_fork_offset;
uint64_t last_out_offset;
/* exported for dmg_open */
uint32_t max_compressed_size;
@@ -203,6 +203,8 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
size_t new_size;
uint32_t chunk_count;
int64_t offset = 0;
+uint64_t data_offset;
+uint64_t in_offset = ds->data_fork_offset;
type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
@@ -211,8 +213,12 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
return 0;
}
-offset += 4;
-offset += 200;
+/* location in data fork for (compressed) blob (in bytes) */
+data_offset = buff_read_uint64(buffer, offset + 0x18);
+in_offset += data_offset;
+
+/* move to begin of chunk entries */
+offset += 204;
chunk_count = (count - 204) / 40;
new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
@@ -228,7 +234,6 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
if (s->types[i] != 0x8005 && s->types[i] != 1 &&
s->types[i] != 2) {
if (s->types[i] == 0x && i > 0) {
-ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
ds->last_out_offset = s->sectors[i - 1] +
s->sectorcounts[i - 1];
}
@@ -255,7 +260,7 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
}
s->offsets[i] = buff_read_uint64(buffer, offset);
-s->offsets[i] += ds->last_in_offset;
+s->offsets[i] += in_offset;
offset += 8;
s->lengths[i] = buff_read_uint64(buffer, offset);
@@ -413,7 +418,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
/* used by dmg_read_mish_block to keep track of the current I/O position */
-ds.last_in_offset = 0;
+ds.data_fork_offset = 0;
ds.last_out_offset = 0;
ds.max_compressed_size = 1;
ds.max_sectors_per_chunk = 1;
@@ -425,6 +430,15 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
goto fail;
}
+/* offset of data fork (DataForkOffset) */
+ret = read_uint64(bs, offset + 0x18, &ds.data_fork_offset);
+if (ret < 0) {
+goto fail;
+} else if (ds.data_fork_offset > offset) {
+ret = -EINVAL;
+goto fail;
+}
+
/* offset of resource fork (RsrcForkOffset) */
ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
if (ret < 0) {
--
2.2.1
[Qemu-devel] [PATCH v2 07/12] block/dmg: set virtual size to a non-zero value
Right now the virtual size is always reported as zero which makes it
impossible to convert between formats.
After this patch, the number of sectors will be read from the trailer
("koly" block).
To verify the behavior, the output of `dmg2img foo.dmg foo.img` was
compared against `qemu-img convert -f dmg -O raw foo.dmg foo.raw`. The
tests showed that the file contents are exactly the same, except that
QEMU creates a slightly larger file (it matches the total sectors
count).
Signed-off-by: Peter Wu
---
v2: fixed typo in commit message (s/mish/koly/)
---
block/dmg.c | 8
1 file changed, 8 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index 1a0fa0e..57feb1b 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -453,6 +453,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
ret = -EINVAL;
goto fail;
}
+ret = read_uint64(bs, offset + 0x1ec, (uint64_t *)&bs->total_sectors);
+if (ret < 0) {
+goto fail;
+}
+if (bs->total_sectors < 0) {
+ret = -EINVAL;
+goto fail;
+}
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
--
2.2.1
[Qemu-devel] [PATCH v2 06/12] block/dmg: process XML plists
The format is simple enough to avoid using a full-blown XML parser. It
assumes that all BLKX items begin with the "mish" magic word, therefore
it is not a problem if other values get matched which are not a BLKX
block.
The offsets are based on the description at
http://newosxbook.com/DMG.html
Signed-off-by: Peter Wu
---
v2: added offset check, allow resource fork to be located at beginning
of file (removed `rsrc_fork_offset != 0` condition)
It got a `Reviewed-by: John Snow ` for v1. I have not
copied the R-b as I wanted to be sure that John agrees with the removal
of the offset != 0 condition.
---
block/dmg.c | 74 +
1 file changed, 74 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index 5f6976b..1a0fa0e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -26,6 +26,7 @@
#include "qemu/bswap.h"
#include "qemu/module.h"
#include
+#include
enum {
/* Limit chunk sizes to prevent unreasonable amounts of memory being used
@@ -345,12 +346,66 @@ fail:
return ret;
}
+static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
+ uint64_t info_begin, uint64_t info_length)
+{
+BDRVDMGState *s = bs->opaque;
+int ret;
+uint8_t *buffer = NULL;
+char *data_begin, *data_end;
+
+/* Have at least some length to avoid NULL for g_malloc. Attempt to set a
+ * safe upper cap on the data length. A test sample had a XML length of
+ * about 1 MiB. */
+if (info_length == 0 || info_length > 16 * 1024 * 1024) {
+ret = -EINVAL;
+goto fail;
+}
+
+buffer = g_malloc(info_length + 1);
+buffer[info_length] = '\0';
+ret = bdrv_pread(bs->file, info_begin, buffer, info_length);
+if (ret != info_length) {
+ret = -EINVAL;
+goto fail;
+}
+
+/* look for The data is 284 (0x11c) bytes after base64
+ * decode. The actual data element has 431 (0x1af) bytes which includes
tabs
+ * and line feeds. */
+data_end = (char *)buffer;
+while ((data_begin = strstr(data_end, "")) != NULL) {
+gsize out_len = 0;
+
+data_begin += 6;
+data_end = strstr(data_begin, "");
+/* malformed XML? */
+if (data_end == NULL) {
+ret = -EINVAL;
+goto fail;
+}
+*data_end++ = '\0';
+g_base64_decode_inplace(data_begin, &out_len);
+ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
+ (uint32_t)out_len);
+if (ret < 0) {
+goto fail;
+}
+}
+ret = 0;
+
+fail:
+g_free(buffer);
+return ret;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
BDRVDMGState *s = bs->opaque;
DmgHeaderState ds;
uint64_t rsrc_fork_offset, rsrc_fork_length;
+uint64_t plist_xml_offset, plist_xml_length;
int64_t offset;
int ret;
@@ -384,12 +439,31 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
ret = -EINVAL;
goto fail;
}
+/* offset of property list (XMLOffset) */
+ret = read_uint64(bs, offset + 0xd8, &plist_xml_offset);
+if (ret < 0) {
+goto fail;
+}
+ret = read_uint64(bs, offset + 0xe0, &plist_xml_length);
+if (ret < 0) {
+goto fail;
+}
+if (plist_xml_offset >= offset ||
+plist_xml_length > offset - plist_xml_offset) {
+ret = -EINVAL;
+goto fail;
+}
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
if (ret < 0) {
goto fail;
}
+} else if (plist_xml_length != 0) {
+ret = dmg_read_plist_xml(bs, &ds, plist_xml_offset, plist_xml_length);
+if (ret < 0) {
+goto fail;
+}
} else {
ret = -EINVAL;
goto fail;
--
2.2.1
[Qemu-devel] [PATCH v2 03/12] block/dmg: extract processing of resource forks
Besides the offset, also read the resource length. This length is now
used in the extracted function to verify the end of the resource fork
against "count" from the resource fork.
Instead of relying on the value of offset to conclude whether the
resource fork is available or not (info_begin==0), check the
rsrc_fork_length instead. This would allow a dmg file to begin with a
resource fork. This seemingly unnecessary restriction was found while
trying to craft a DMG file by hand.
Other changes:
- Do not require resource data offset to be 0x100 (but check that it
is within bounds though).
- Further improve boundary checking (resource data must be within
the resource fork).
- Use correct value for resource data length (spotted by John Snow)
- Consider the resource data offset when determining info_end.
This fixes an EINVAL on the tuxpaint dmg example.
The resource fork format is documented at
https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151
Signed-off-by: Peter Wu
---
v2: expanded commit message (incl. documentation link). Do not require
a fixed resource data offset of 0x100, improve boundary checking,
fix resource data len (8 vs 4), append offset when checking the end
of the resource data.
--
block/dmg.c | 104 ++--
1 file changed, 66 insertions(+), 38 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index e01559f..ed99cf5 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -287,60 +287,38 @@ fail:
return ret;
}
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
-Error **errp)
+static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
+ uint64_t info_begin, uint64_t info_length)
{
-BDRVDMGState *s = bs->opaque;
-DmgHeaderState ds;
-uint64_t info_begin, info_end;
-uint32_t count, rsrc_data_offset;
-int64_t offset;
int ret;
+uint32_t count, rsrc_data_offset;
+uint64_t info_end;
+uint64_t offset;
-bs->read_only = 1;
-s->n_chunks = 0;
-s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
-/* used by dmg_read_mish_block to keep track of the current I/O position */
-ds.last_in_offset = 0;
-ds.last_out_offset = 0;
-ds.max_compressed_size = 1;
-ds.max_sectors_per_chunk = 1;
-
-/* locate the UDIF trailer */
-offset = dmg_find_koly_offset(bs->file, errp);
-if (offset < 0) {
-ret = offset;
-goto fail;
-}
-
-ret = read_uint64(bs, offset + 0x28, &info_begin);
-if (ret < 0) {
-goto fail;
-} else if (info_begin == 0) {
-ret = -EINVAL;
-goto fail;
-}
-
+/* read offset from begin of resource fork (info_begin) to resource data */
ret = read_uint32(bs, info_begin, &rsrc_data_offset);
if (ret < 0) {
goto fail;
-} else if (rsrc_data_offset != 0x100) {
+} else if (rsrc_data_offset > info_length) {
ret = -EINVAL;
goto fail;
}
-ret = read_uint32(bs, info_begin + 4, &count);
+/* read length of resource data */
+ret = read_uint32(bs, info_begin + 8, &count);
if (ret < 0) {
goto fail;
-} else if (count == 0) {
+} else if (count == 0 || rsrc_data_offset + count > info_length) {
ret = -EINVAL;
goto fail;
}
-/* end of resource data, ignoring the following resource map */
-info_end = info_begin + count;
/* begin of resource data (consisting of one or more resources) */
-offset = info_begin + 0x100;
+offset = info_begin + rsrc_data_offset;
+
+/* end of resource data (there is possibly a following resource map
+ * which will be ignored). */
+info_end = offset + count;
/* read offsets (mish blocks) from one or more resources in resource data
*/
while (offset < info_end) {
@@ -354,13 +332,63 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
}
offset += 4;
-ret = dmg_read_mish_block(bs, &ds, offset, count);
+ret = dmg_read_mish_block(bs, ds, offset, count);
if (ret < 0) {
goto fail;
}
/* advance offset by size of resource */
offset += count;
}
+return 0;
+
+fail:
+return ret;
+}
+
+static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
+Error **errp)
+{
+BDRVDMGState *s = bs->opaque;
+DmgHeaderState ds;
+uint64_t rsrc_fork_offset, rsrc_fork_length;
+int64_t offset;
+int ret;
+
+bs->read_only = 1;
+s->n_chunks = 0;
+s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
+/* used by dmg_read_mish_block to keep track of the current I/O position */
+ds.last_in_offset = 0;
+ds.last_out_offset = 0;
+ds.max_compressed_size = 1;
+ds.max_sectors_per_chunk = 1;
+
+/* locate the UDIF trailer */
+
[Qemu-devel] [PATCH v2 10/12] block/dmg: factor out block type check
In preparation for adding bzip2 support, split the type check into a
separate function. Make all offsets relative to the begin of a chunk
such that it is easier to recognize the position without having to
add up all offsets. Some comments are added to describe the fields.
There is no functional change.
Signed-off-by: Peter Wu
---
v2: new patch, split off bzip2 patch. Besides the
dmg_is_known_block_type function, the offsets are now also made
relative.
---
block/dmg.c | 36 +++-
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 57922c5..b1d0930 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -194,6 +194,18 @@ typedef struct DmgHeaderState {
uint32_t max_sectors_per_chunk;
} DmgHeaderState;
+static bool dmg_is_known_block_type(uint32_t entry_type)
+{
+switch (entry_type) {
+case 0x0001:/* uncompressed */
+case 0x0002:/* zeroes */
+case 0x8005:/* zlib */
+return true;
+default:
+return false;
+}
+}
+
static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
uint8_t *buffer, uint32_t count)
{
@@ -233,22 +245,19 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
s->types[i] = buff_read_uint32(buffer, offset);
-offset += 4;
-if (s->types[i] != 0x8005 && s->types[i] != 1 &&
-s->types[i] != 2) {
+if (!dmg_is_known_block_type(s->types[i])) {
chunk_count--;
i--;
-offset += 36;
+offset += 40;
continue;
}
-offset += 4;
-s->sectors[i] = buff_read_uint64(buffer, offset);
+/* sector number */
+s->sectors[i] = buff_read_uint64(buffer, offset + 8);
s->sectors[i] += out_offset;
-offset += 8;
-s->sectorcounts[i] = buff_read_uint64(buffer, offset);
-offset += 8;
+/* sector count */
+s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
error_report("sector count %" PRIu64 " for chunk %" PRIu32
@@ -258,12 +267,12 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
goto fail;
}
-s->offsets[i] = buff_read_uint64(buffer, offset);
+/* offset in (compressed) data fork */
+s->offsets[i] = buff_read_uint64(buffer, offset + 0x18);
s->offsets[i] += in_offset;
-offset += 8;
-s->lengths[i] = buff_read_uint64(buffer, offset);
-offset += 8;
+/* length in (compressed) data fork */
+s->lengths[i] = buff_read_uint64(buffer, offset + 0x20);
if (s->lengths[i] > DMG_LENGTHS_MAX) {
error_report("length %" PRIu64 " for chunk %" PRIu32
@@ -275,6 +284,7 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
update_max_chunk_size(s, i, &ds->max_compressed_size,
&ds->max_sectors_per_chunk);
+offset += 40;
}
s->n_chunks += chunk_count;
return 0;
--
2.2.1
[Qemu-devel] [PATCH v2 12/12] block/dmg: improve zeroes handling
Disk images may contain large all-zeroes gaps (1.66k sectors or 812 MiB
is seen in the real world). These blocks (type 2) do not need to be
extracted into a temporary buffer, there is no need to allocate memory
for these blocks nor to check its length.
(For the test image, the maximum uncompressed size is 1054371 bytes,
probably for a bzip2-compressed block.)
Signed-off-by: Peter Wu
---
v2: no changes (did not receive comments last time)
---
block/dmg.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 8239221..4e24076 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -137,7 +137,9 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t
chunk,
uncompressed_sectors = (s->lengths[chunk] + 511) / 512;
break;
case 2: /* zero */
-uncompressed_sectors = s->sectorcounts[chunk];
+/* as the all-zeroes block may be large, it is treated specially: the
+ * sector is not copied from a large buffer, a simple memset is used
+ * instead. Therefore uncompressed_sectors does not need to be set. */
break;
}
@@ -269,7 +271,9 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
/* sector count */
s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
-if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+/* all-zeroes sector (type 2) does not need to be "uncompressed" and
can
+ * therefore be unbounded. */
+if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
error_report("sector count %" PRIu64 " for chunk %" PRIu32
" is larger than max (%u)",
s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -644,7 +648,8 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
}
break;
case 2: /* zero */
-memset(s->uncompressed_chunk, 0, 512 * s->sectorcounts[chunk]);
+/* see dmg_read, it is treated specially. No buffer needs to be
+ * pre-filled, the zeroes can be set directly. */
break;
}
s->current_chunk = chunk;
@@ -663,6 +668,13 @@ static int dmg_read(BlockDriverState *bs, int64_t
sector_num,
if (dmg_read_chunk(bs, sector_num + i) != 0) {
return -1;
}
+/* Special case: current chunk is all zeroes. Do not perform a memcpy
as
+ * s->uncompressed_chunk may be too small to cover the large all-zeroes
+ * section. dmg_read_chunk is called to find s->current_chunk */
+if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
+memset(buf + i * 512, 0, 512);
+continue;
+}
sector_offset_in_chunk = sector_num + i - s->sectors[s->current_chunk];
memcpy(buf + i * 512,
s->uncompressed_chunk + sector_offset_in_chunk * 512, 512);
--
2.2.1
[Qemu-devel] [PATCH v2 02/12] block/dmg: extract mish block decoding functionality
Extract the mish block decoder such that this can be used for other
formats in the future. A new DmgHeaderState struct is introduced to
share state while decoding.
The code is kept unchanged as much as possible, a "fail" label is added
for example where a simple return would probably do. In dmg_open, the
variable "tmp" is renamed to "rsrc_data_offset" for clarity and comments
have been added explaining various data.
Note that this patch has one subtle difference with the previous
version which should not affect functionality. In the previous code,
the end of a resource was inferred from the mish block (the offsets
would be increased by the fields). In this patch, the resource length
is used instead to avoid the need to rely on the previous offsets.
Signed-off-by: Peter Wu
Reviewed-by: John Snow
Reviewed-by: Stefan Hajnoczi
---
v2: added R-b, added comments, renamed "tmp" var to "rsrc_data_offset".
--
block/dmg.c | 228 +++-
1 file changed, 133 insertions(+), 95 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 9183459..e01559f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -173,19 +173,138 @@ static int64_t dmg_find_koly_offset(BlockDriverState
*file_bs, Error **errp)
return -EINVAL;
}
+/* used when building the sector table */
+typedef struct DmgHeaderState {
+/* used internally by dmg_read_mish_block to remember offsets of blocks
+ * across calls */
+uint64_t last_in_offset;
+uint64_t last_out_offset;
+/* exported for dmg_open */
+uint32_t max_compressed_size;
+uint32_t max_sectors_per_chunk;
+} DmgHeaderState;
+
+static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
+ int64_t offset, uint32_t count)
+{
+BDRVDMGState *s = bs->opaque;
+uint32_t type, i;
+int ret;
+size_t new_size;
+uint32_t chunk_count;
+
+ret = read_uint32(bs, offset, &type);
+if (ret < 0) {
+goto fail;
+}
+
+/* skip data that is not a valid MISH block (invalid magic or too small) */
+if (type != 0x6d697368 || count < 244) {
+/* assume success for now */
+return 0;
+}
+
+offset += 4;
+offset += 200;
+
+chunk_count = (count - 204) / 40;
+new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
+s->types = g_realloc(s->types, new_size / 2);
+s->offsets = g_realloc(s->offsets, new_size);
+s->lengths = g_realloc(s->lengths, new_size);
+s->sectors = g_realloc(s->sectors, new_size);
+s->sectorcounts = g_realloc(s->sectorcounts, new_size);
+
+for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
+ret = read_uint32(bs, offset, &s->types[i]);
+if (ret < 0) {
+goto fail;
+}
+offset += 4;
+if (s->types[i] != 0x8005 && s->types[i] != 1 &&
+s->types[i] != 2) {
+if (s->types[i] == 0x && i > 0) {
+ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
+ds->last_out_offset = s->sectors[i - 1] +
+ s->sectorcounts[i - 1];
+}
+chunk_count--;
+i--;
+offset += 36;
+continue;
+}
+offset += 4;
+
+ret = read_uint64(bs, offset, &s->sectors[i]);
+if (ret < 0) {
+goto fail;
+}
+s->sectors[i] += ds->last_out_offset;
+offset += 8;
+
+ret = read_uint64(bs, offset, &s->sectorcounts[i]);
+if (ret < 0) {
+goto fail;
+}
+offset += 8;
+
+if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+error_report("sector count %" PRIu64 " for chunk %" PRIu32
+ " is larger than max (%u)",
+ s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
+ret = -EINVAL;
+goto fail;
+}
+
+ret = read_uint64(bs, offset, &s->offsets[i]);
+if (ret < 0) {
+goto fail;
+}
+s->offsets[i] += ds->last_in_offset;
+offset += 8;
+
+ret = read_uint64(bs, offset, &s->lengths[i]);
+if (ret < 0) {
+goto fail;
+}
+offset += 8;
+
+if (s->lengths[i] > DMG_LENGTHS_MAX) {
+error_report("length %" PRIu64 " for chunk %" PRIu32
+ " is larger than max (%u)",
+ s->lengths[i], i, DMG_LENGTHS_MAX);
+ret = -EINVAL;
+goto fail;
+}
+
+update_max_chunk_size(s, i, &ds->max_compressed_size,
+ &ds->max_sectors_per_chunk);
+}
+s->n_chunks += chunk_count;
+return 0;
+
+fail:
+return ret;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
BDRVDMGState *s = bs->opaque;
-uint64_t info_begin, info_end, last_in_offset, last_out_offset;
-
Re: [Qemu-devel] [PATCH v8 2/3] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier
On 5 January 2015 at 16:14, Eric Auger wrote:
> Device tree nodes for the platform bus and its children dynamic sysbus
> devices are added in a machine init done notifier. To load the dtb once,
> after those latter nodes are built and before ROM freeze, the actual
> arm_load_kernel existing code is moved into a notifier notify function,
> arm_load_kernel_notify. arm_load_kernel now only registers the
> corresponding notifier.
>
> Machine files that do not support platform bus stay unchanged. Machine
> files willing to support dynamic sysbus devices must call arm_load_kernel
> before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
> dynamic sysbus device nodes are integrated in the dtb.
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -19,6 +19,16 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
>int flash_size, int sram_size,
>const char *kernel_filename, const char *cpu_model);
>
> +/*
> + * struct used as a parameter of the arm_load_kernel machine init
> + * done notifier
> + */
> +typedef struct {
> +ARMCPU *cpu; /* handle to the first cpu object */
> +Notifier notifier; /* actual notifier */
This is an odd order -- why not put 'notifier' first?
> +} ArmLoadKernelNotifier;
> +
> +
This breaks building of the linux-user targets:
In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/target-arm/cpu.c:29:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/include/hw/arm/arm.h:28:5:
error: unknown type name ‘Notifier’
Notifier notifier; /* actual notifier */
^
make[1]: *** [target-arm/cpu.o] Error 1
Including "qemu/notify.h" in this file fixes it.
Also, stray blank line.
Otherwise looks OK. (It seems a slight shame to have to drag
notify.h into everything that includes arm.h when really the
notifier is just a detail of the implementation of boot.c,
but I can't see a nice way round that, so never mind.)
-- PMM
[Qemu-devel] [PATCH v2 11/12] block/dmg: support bzip2 block entry types
This patch adds support for bzip2-compressed block entries as introduced
with OS X 10.4 (source: https://en.wikipedia.org/wiki/Apple_Disk_Image).
It was tested against a 5.2G "OS X Yosemite" installation image which
stores the BLXX block in the XML property list (instead of resource
forks) and has over 5k chunks.
New configure entries are added (--enable-bzip2 / --disable-bzip2) to
control inclusion of bzip2 functionality (which requires linking against
libbz2). The help message suggests that this option is needed for DMG
files, but the tests are generic enough that other parts of QEMU can use
bzip2 if needed.
The identifiers are based on http://newosxbook.com/DMG.html.
The decompression routines are based on the zlib case, but as there is
no way to reset the decompression state (unlike zlib), memory is
allocated and deallocated for every decompression. This should not be
problematic as the decompression takes most of the time and as blocks
are typically about/over 1 MiB in size, only one allocation is done
every 2000 sectors.
Signed-off-by: Peter Wu
---
v2: split block type check into a different patch ("[PATCH v2 10/12]
block/dmg: factor out block type check").
Add BZIP2_LIBS instead of polluting libs_softmmu with -lbz2 (which
would also end up in fsdev/virtfs-proxy-helper).
Fix unused variable warning with --disable-bzip2.
---
block/Makefile.objs | 1 +
block/dmg.c | 43 ++-
configure | 31 +++
3 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 04b0e43..e7ea07c 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -36,5 +36,6 @@ gluster.o-libs := $(GLUSTERFS_LIBS)
ssh.o-cflags := $(LIBSSH2_CFLAGS)
ssh.o-libs := $(LIBSSH2_LIBS)
archipelago.o-libs := $(ARCHIPELAGO_LIBS)
+dmg.o-libs := $(BZIP2_LIBS)
qcow.o-libs:= -lz
linux-aio.o-libs := -laio
diff --git a/block/dmg.c b/block/dmg.c
index b1d0930..8239221 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -26,6 +26,9 @@
#include "qemu/bswap.h"
#include "qemu/module.h"
#include
+#ifdef CONFIG_BZIP2
+#include
+#endif
#include
enum {
@@ -56,6 +59,9 @@ typedef struct BDRVDMGState {
uint8_t *compressed_chunk;
uint8_t *uncompressed_chunk;
z_stream zstream;
+#ifdef CONFIG_BZIP2
+bz_stream bzstream;
+#endif
} BDRVDMGState;
static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -123,6 +129,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t
chunk,
switch (s->types[chunk]) {
case 0x8005: /* zlib compressed */
+case 0x8006: /* bzip2 compressed */
compressed_size = s->lengths[chunk];
uncompressed_sectors = s->sectorcounts[chunk];
break;
@@ -200,6 +207,9 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
case 0x0001:/* uncompressed */
case 0x0002:/* zeroes */
case 0x8005:/* zlib */
+#ifdef CONFIG_BZIP2
+case 0x8006:/* bzip2 */
+#endif
return true;
default:
return false;
@@ -565,13 +575,16 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
if (!is_sector_in_chunk(s, s->current_chunk, sector_num)) {
int ret;
uint32_t chunk = search_chunk(s, sector_num);
+#ifdef CONFIG_BZIP2
+uint64_t total_out;
+#endif
if (chunk >= s->n_chunks) {
return -1;
}
s->current_chunk = s->n_chunks;
-switch (s->types[chunk]) {
+switch (s->types[chunk]) { /* block entry type */
case 0x8005: { /* zlib compressed */
/* we need to buffer, because only the chunk as whole can be
* inflated. */
@@ -595,6 +608,34 @@ static inline int dmg_read_chunk(BlockDriverState *bs,
uint64_t sector_num)
return -1;
}
break; }
+#ifdef CONFIG_BZIP2
+case 0x8006: /* bzip2 compressed */
+/* we need to buffer, because only the chunk as whole can be
+ * inflated. */
+ret = bdrv_pread(bs->file, s->offsets[chunk],
+ s->compressed_chunk, s->lengths[chunk]);
+if (ret != s->lengths[chunk]) {
+return -1;
+}
+
+ret = BZ2_bzDecompressInit(&s->bzstream, 0, 0);
+if (ret != BZ_OK) {
+return -1;
+}
+s->bzstream.next_in = (char *)s->compressed_chunk;
+s->bzstream.avail_in = (unsigned int) s->lengths[chunk];
+s->bzstream.next_out = (char *)s->uncompressed_chunk;
+s->bzstream.avail_out = (unsigned int) 512 *
s->sectorcounts[chunk];
+ret = BZ2_bzDecompress(&s->bzstream);
+total_out = ((uint64_t)s->bzstream.total_out_hi32 << 32) +
+s->bzstream.total_out_lo32;
+
Re: [Qemu-devel] [PATCH] block: limited request size in write zeroes unsupported path
On 06/01/15 18:43, Stefan Hajnoczi wrote: On Mon, Jan 05, 2015 at 03:34:07PM +0300, Denis V. Lunev wrote: Though pls consider my patch v3, it avoids allocation of 16 Mb here and uses only 1 Mb of memory. Once your patch has Reviewed-by: it will show up on my radar for merge. If you and Peter need a 2nd opinion in your discussions about the fallocate series, I can look at the series in more detail myself. Just let me know. Stefan Fallocate stuff has been reviewed by Fam and I have enough feedback at the moment to start rework. He wants some simplifications at the moment. This is not a big deal. This patch is technically correct and solves the problem I have spotted. Thus it could be merged. I'll drop patch 1 in my series for the sake of this one to avoid unnecessary discussion with it. On the other hand I believe that my patch is a little bit better, it allocates only 1 MB instead of 16 here. Though I could rebase it and send it separately on top of this to discuss it independently. By the way, Stefan, do you see Acked-by: tag in your radar or it should be avoided? We are using it as review signature thanks to my prior Linux kernel experience. Regards, Den
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
On Jan 6, 2015, at 12:30 PM, Peter Maydell wrote: > On 6 January 2015 at 17:19, Programmingkid wrote: >> After investigating the TARGET_WORDS_BIGENDIAN code, I noticed >> that s->default_endian_fb was being set to true. So I undefined >> the macro and then ran QEMU. The i386 target showed no change >> in colors. The ppc target still had the same incorrect colors. >> Sorry, this doesn't fix the problem. > > What macro? You don't want to undefine TARGET_WORDS_BIGENDIAN, > that will wreak all kinds of havoc. Just test with manually > setting default_endian_fb to false. > > -- PMM Just tried that. It didn't fix the color problem. Here are the test I have done so far: Experiment: Disable preprocessor code so that thebool byteswap = !s->big_endian_fb code is used. Line 1440. Result: No change in colors for Mac OS X guest. They are still incorrect. Experiment: Set the HOST_WORDS_BIGENDIAN macro at line 94. Result: Did not change the colors. They are still messed up. It made all the colors in the PC emulator look weird - like a Nintendo cartridge not fully seated correctly. But it then changed back to the normal color layout. Experiment: undefine TARGET_WORDS_BIGENDIAN at line 2155 in vga.c. Result: PC emulator shows no change in colors - good. Mac emulator shows the same incorrect colors -bad. Experiment: remove the ! from byteswap in vga.c:1495. Result: The colors are still incorrect in qemu-system-ppc with a Mac OS X guest. Colors are unaffected in the i386 target. Experiment: Just manually set default_endian_fb to false. Result: Did not fix the problem. Colors still show up incorrectly in the ppc emulator. Sorry but I don't think this is a bug with QEMU. I think the pixel format is suppose to be in the little endian format. IBM developed this standard for this PCs which used little endian x86 processors. Why would they want to spend the time and money on making their vga cards work on non-pc/2 compatible computers? They simply had no reason to support the big endian format.
Re: [Qemu-devel] Gives user ability to select endian format for video display - fixes Mac OS X guest color issue.
https://opensource.apple.com/source/IOGraphics/IOGraphics-45.3/IOGraphicsFamily/IOBootFramebuffer.cpp This file is used for the frame buffer in Mac OS 10.2. There is no mention of the endian format for the pixels. That seems to indicate an oversight on Apple's part. http://www.mcamafia.de/pdf/ibm_vgaxga_trm2.pdf This file is the specifications to the VGA standard. It makes no mention of pixel endian format. There is no mention of bit order in the specifications. It's probably assumed to be little endian.
