Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation
On 08/18/2014 11:54 PM, Joel Schopp wrote: > > +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque) > +{ > +PlatformDevtreeData *data = opaque; > +void *fdt = data->fdt; > +const char *parent_node = data->node; > +int compat_str_len; > +char *nodename; > +int i, ret; > +uint32_t *irq_attr; > +uint64_t *reg_attr; > +uint64_t mmio_base; > +uint64_t irq_number; > +gchar mmio_base_prop[8]; > +gchar irq_number_prop[8]; > +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); > +VFIODevice *vbasedev = &vdev->vbasedev; > +Object *obj = OBJECT(sbdev); > + > +mmio_base = object_property_get_int(obj, "mmio[0]", NULL); > + > +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node, > + vbasedev->name, > + mmio_base); > + > +qemu_fdt_add_subnode(fdt, nodename); > + > +compat_str_len = strlen(vdev->compat) + 1; > +qemu_fdt_setprop(fdt, nodename, "compatible", > +vdev->compat, compat_str_len); > + > +reg_attr = g_new(uint64_t, vbasedev->num_regions*4); > + > +for (i = 0; i < vbasedev->num_regions; i++) { > +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i); > +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL); > +reg_attr[2*i] = 1; > +reg_attr[2*i+1] = mmio_base; > +reg_attr[2*i+2] = 1; > +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem); > +} > > This should be 4 instead of 2. Hi Joel, Yes definitively! Forgot to restore the original value after trying different qemu_fdt_setprop_* functions. sorry for that. Best Regards Eric > Also, to support 64 bit systems I think this should be 2 instead of 1. >
Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *
Max Reitz writes: > On 18.08.2014 18:10, Markus Armbruster wrote: >> They clutter the code. Unfortunately, I can't figure out how to make >> Coccinelle drop all of them, so I have to settle for common special >> cases: >> >> @@ >> type T; >> T *pt; >> void *pv; >> @@ >> - pt = (T *)pv; >> + pt = pv; >> @@ >> type T; >> @@ >> - (T *) >>(\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| >> g_try_malloc\|g_try_malloc0\|g_try_realloc\| >> g_try_new\|g_try_new0\|g_try_renew\)(...)) >> >> Signed-off-by: Markus Armbruster >> --- >> block/vhdx-log.c| 2 +- >> block/vvfat.c | 8 >> hw/ide/microdrive.c | 2 +- >> 3 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/block/vhdx-log.c b/block/vhdx-log.c >> index eb5c7a0..4267e60 100644 >> --- a/block/vhdx-log.c >> +++ b/block/vhdx-log.c >> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, >> BDRVVHDXState *s, >> buffer = qemu_blockalign(bs, total_length); >> memcpy(buffer, &new_hdr, sizeof(new_hdr)); >> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); >> +new_desc = (buffer + sizeof(new_hdr)); >> data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE); >> data_tmp = data; > > You could drop the parantheses here. Also, I still don't like void > pointer arithmetic, but well... ;-) Separate issue, related because it would put the cast right back. We use plenty of GCCisms. If we ever want to port to a toolchain that doesn't support them, adding the cast clutter to avoid arithmetic on void * will be the least of our worries. If we decide to hunt down this particular GCCism anyway, there's -Wpointer-arith. Without that, it's a game of whack-a-mole. > Reviewed-by: Max Reitz Thanks!
Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *
Jeff Cody writes: > On Mon, Aug 18, 2014 at 06:10:43PM +0200, Markus Armbruster wrote: >> They clutter the code. Unfortunately, I can't figure out how to make >> Coccinelle drop all of them, so I have to settle for common special >> cases: >> >> @@ >> type T; >> T *pt; >> void *pv; >> @@ >> - pt = (T *)pv; >> + pt = pv; >> @@ >> type T; >> @@ >> - (T *) >> (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| >> g_try_malloc\|g_try_malloc0\|g_try_realloc\| >> g_try_new\|g_try_new0\|g_try_renew\)(...)) >> >> Signed-off-by: Markus Armbruster >> --- >> block/vhdx-log.c| 2 +- >> block/vvfat.c | 8 >> hw/ide/microdrive.c | 2 +- >> 3 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/block/vhdx-log.c b/block/vhdx-log.c >> index eb5c7a0..4267e60 100644 >> --- a/block/vhdx-log.c >> +++ b/block/vhdx-log.c >> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, >> BDRVVHDXState *s, >> buffer = qemu_blockalign(bs, total_length); >> memcpy(buffer, &new_hdr, sizeof(new_hdr)); >> >> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); >> +new_desc = (buffer + sizeof(new_hdr)); > > Agree with Max, in that the parenthesis could be removed. Not worthy > of a respin normally, but since the point of this patch is to > unclutter the code, I guess it makes sense to fix it. Max and you are right. I neglected to clean up the patch produced by spatch. >> data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE); >> data_tmp = data; >> >> diff --git a/block/vvfat.c b/block/vvfat.c >> index f877e85..401539d 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int >> mapping_index) >> if(first_cluster == 0 && (is_dotdot || is_dot)) >> continue; >> >> -buffer=(char*)g_malloc(length); >> +buffer=g_malloc(length); > > You missed a spot to put spaces around the '=' here. Nothing worthy > of a respin, of course - just a note in case you decide to respin. Right again. Although perhaps vvfat should remain ugly, to warn unwary travellers ;) > [...] > > With or without the changes above: > > Reviewed-by: Jeff Cody Thanks!
Re: [Qemu-devel] [PATCH 1/3] docs: List all image elements currently supported by the fuzzer
On Mon, 08/11 15:55, Maria Kustova wrote: > Signed-off-by: Maria Kustova > --- > docs/image-fuzzer.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt > index 0d0005d..f707269 100644 > --- a/docs/image-fuzzer.txt > +++ b/docs/image-fuzzer.txt > @@ -125,7 +125,8 @@ If a fuzzer configuration is specified, then it has the > next interpretation: > will be always fuzzed for every test. This case is useful for regression > testing. > > -For now only header fields, header extensions and L1/L2 tables are generated. > +The generator can create header fields, header extensions, L1/L2 tables and > +refcount blocks and table. "refcount table and blocks" might read better, but it doesn't hurt. Reviewed-by: Fam Zheng > > Module interfaces > - > -- > 1.9.3 >
Re: [Qemu-devel] [Qemu-trivial] [PATCH v6 03/10] virtio-blk: fix reference a pointer which might be freed
19.08.2014 00:17, Michael S. Tsirkin wrote: [] > By the way, could you please add Cc qemu-stable on bugfixes > you have queued? > These are likely appopriate for 2.1.1. Actually I've added Cc: qemu-stable@ in the commit message. So it will go to stable (or should) once I'll send a pull request. Thanks, /mjt
[Qemu-devel] [PATCH] pcihp: fix possible array out of bounds
From: Gonglei When 'bsel == ACPI_PCIHP_MAX_HOTPLUG_BUS', the s->acpi_pcihp_pci_status[bsel] array will out of bounds. Add check for this. Signed-off-by: Gonglei --- hw/acpi/pcihp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index fae663a..34dedf1 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -231,7 +231,7 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size) uint32_t val = 0; int bsel = s->hotplug_select; -if (bsel < 0 || bsel > ACPI_PCIHP_MAX_HOTPLUG_BUS) { +if (bsel < 0 || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) { return 0; } -- 1.7.12.4
Re: [Qemu-devel] [PATCH] configure: no need to mkdir QMP
Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation
On 08/19/2014 12:11 AM, Peter Maydell wrote: > On 18 August 2014 22:54, Joel Schopp wrote: >> >> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque) >> +{ >> +PlatformDevtreeData *data = opaque; >> +void *fdt = data->fdt; >> +const char *parent_node = data->node; >> +int compat_str_len; >> +char *nodename; >> +int i, ret; >> +uint32_t *irq_attr; >> +uint64_t *reg_attr; >> +uint64_t mmio_base; >> +uint64_t irq_number; >> +gchar mmio_base_prop[8]; >> +gchar irq_number_prop[8]; >> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); >> +VFIODevice *vbasedev = &vdev->vbasedev; >> +Object *obj = OBJECT(sbdev); >> + >> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL); >> + >> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node, >> + vbasedev->name, >> + mmio_base); >> + >> +qemu_fdt_add_subnode(fdt, nodename); >> + >> +compat_str_len = strlen(vdev->compat) + 1; > > At this point you've already substituted the NULs in, > so you can't call strlen(), I think. Hi Peter, yes you're right. Thanks > >> +qemu_fdt_setprop(fdt, nodename, "compatible", >> +vdev->compat, compat_str_len); >> + >> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4); >> + >> +for (i = 0; i < vbasedev->num_regions; i++) { >> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i); >> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL); >> +reg_attr[2*i] = 1; >> +reg_attr[2*i+1] = mmio_base; >> +reg_attr[2*i+2] = 1; >> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem); >> +} >> >> This should be 4 instead of 2. >> Also, to support 64 bit systems I think this should be 2 instead of 1. > > Actually it depends entirely on what the board has done to > create the device tree node that we're inserting this child > node into. For ARM boot.c sets both #address-cells and > #size-cells to 2 regardless of whether the system is 32 > or 64 bits, for simplicity. I imagine PPC does something > different. If we're editing a dtb that the user passed in (which > I think would be pretty lunatic so we shouldn't do this) > we'd have to actually walk the dtb to try to figure out what > the semantics of the reg property should be. Putting size=1 was the only solution I found to use an offset relative to the parent bus instead of an absolute base address. I would explain this because, in platform_bus_create_devtree, the function that creates the "platform bus" node, #address-cells and #size-cells currently are set to 1. I assume the motivation was that bus size was supposed to be smaller than 4GB. Then I guess the problem is shifted to the inclusion of the platform bus in any ARM platform. Thanks Eric > > thanks > -- PMM >
Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] slirp/misc: Use g_malloc() instead of malloc()
On 2014/8/18 19:32, Michael Tokarev wrote: 18.08.2014 11:51, zhanghailiang пишет: Here we don't check the return value of malloc() which may fail. Use the g_malloc() instead, which will abort the program when there is not enough memory. Signed-off-by: zhanghailiang Reviewed-by: Alex Bennée --- slirp/misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slirp/misc.c b/slirp/misc.c index b8eb74c..f7fe497 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -54,7 +54,7 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char *exec, } tmp_ptr = *ex_ptr; - *ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list)); + *ex_ptr = (struct ex_list *)g_malloc(sizeof(struct ex_list)); There's a convinient macro in glib, g_new(typename, numelts). Also there's a less commonly used g_renew() which is like realloc, but it is not applicable here. Hmm, it is a good idea to use g_new instead of g_malloc, we have to perform type cast for g_malloc.(BTW, i found in qemu there are several places use g_malloc but not perform appropriate type coercions) I will modify this.Thanks :) (*ex_ptr)->ex_fport = port; (*ex_ptr)->ex_addr = addr; (*ex_ptr)->ex_pty = do_pty; @@ -235,7 +235,7 @@ strdup(str) { char *bptr; - bptr = (char *)malloc(strlen(str)+1); + bptr = (char *)g_malloc(strlen(str)+1); strcpy(bptr, str); return bptr; Oh. And this one should be removed completely. It is a reimplementation of strdup() for system which lacks it. This code should go, we don't build I couldn't agree more, i will removed it. on such a system anyway and we always have g_strdup(). There's one more usage of strdup() in this file, btw. OK, Thanks. I'm sorry for being so picky, and you're already at v7, but heck.. We should be more active at reviewing patches :) Aha! ;),i really appreciate your help. I learned a lot. Thanks. Thanks, /mjt .
Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] slirp/misc: Use g_malloc() instead of malloc()
19.08.2014 11:30, zhanghailiang wrote: [] > Hmm, it is a good idea to use g_new instead of g_malloc, > we have to perform type cast for g_malloc.(BTW, i found in qemu there > are several places use g_malloc but not perform appropriate type > coercions) There's no need to perform explicit type conversion from void* to type*. Thanks, /mjt
Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation
On 08/19/2014 12:26 AM, Joel Schopp wrote: > > On 08/18/2014 05:11 PM, Peter Maydell wrote: >> On 18 August 2014 22:54, Joel Schopp wrote: >>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque) >>> +{ >>> +PlatformDevtreeData *data = opaque; >>> +void *fdt = data->fdt; >>> +const char *parent_node = data->node; >>> +int compat_str_len; >>> +char *nodename; >>> +int i, ret; >>> +uint32_t *irq_attr; >>> +uint64_t *reg_attr; >>> +uint64_t mmio_base; >>> +uint64_t irq_number; >>> +gchar mmio_base_prop[8]; >>> +gchar irq_number_prop[8]; >>> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); >>> +VFIODevice *vbasedev = &vdev->vbasedev; >>> +Object *obj = OBJECT(sbdev); >>> + >>> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL); >>> + >>> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node, >>> + vbasedev->name, >>> + mmio_base); >>> + >>> +qemu_fdt_add_subnode(fdt, nodename); >>> + >>> +compat_str_len = strlen(vdev->compat) + 1; >> At this point you've already substituted the NULs in, >> so you can't call strlen(), I think. >> >>> +qemu_fdt_setprop(fdt, nodename, "compatible", >>> +vdev->compat, compat_str_len); >>> + >>> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4); >>> + >>> +for (i = 0; i < vbasedev->num_regions; i++) { >>> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i); >>> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL); >>> +reg_attr[2*i] = 1; >>> +reg_attr[2*i+1] = mmio_base; >>> +reg_attr[2*i+2] = 1; >>> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem); >>> +} >>> >>> This should be 4 instead of 2. >>> Also, to support 64 bit systems I think this should be 2 instead of 1. >> Actually it depends entirely on what the board has done to >> create the device tree node that we're inserting this child >> node into. For ARM boot.c sets both #address-cells and >> #size-cells to 2 regardless of whether the system is 32 >> or 64 bits, for simplicity. I imagine PPC does something >> different. If we're editing a dtb that the user passed in (which >> I think would be pretty lunatic so we shouldn't do this) >> we'd have to actually walk the dtb to try to figure out what >> the semantics of the reg property should be. > For the index [2*i],[2*i+1], etc is clearly a bug as when i = 1 it will > overwrite two of the values. Changing that to [4*i],[4*i+1],etc fixes it. > > I think you are right on the size. I also wonder if the user doesn't > pass in a dtb if qemu should try to recreate the device-tree entry from > the platform device entry in the host kernel? If so would that best be > done by recreating the values from /proc/device-tree ? Antonios recently submitted a patch to retrieve dt info from the vfio platform device. [RFC 0/4] VFIO: PLATFORM: Return device tree info for a platform device node https://www.mail-archive.com/kvm@vger.kernel.org/msg106282.html Best Regards Eric > > I also wish that qemu had a flag to output the generated dtb to a file > much like lkvm (kvmtool) has. >
Re: [Qemu-devel] [PATCH 2/3] fuzz: Add fuzzing functions for entries of refcount table and blocks
On Mon, 08/11 15:55, Maria Kustova wrote: > Signed-off-by: Maria Kustova > --- > tests/image-fuzzer/qcow2/fuzz.py | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/tests/image-fuzzer/qcow2/fuzz.py > b/tests/image-fuzzer/qcow2/fuzz.py > index 57527f9..5852b4d 100644 > --- a/tests/image-fuzzer/qcow2/fuzz.py > +++ b/tests/image-fuzzer/qcow2/fuzz.py > @@ -18,8 +18,8 @@ > > import random > > - > UINT8 = 0xff > +UINT16 = 0x > UINT32 = 0x > UINT64 = 0x > # Most significant bit orders > @@ -28,6 +28,8 @@ UINT64_M = 63 > # Fuzz vectors > UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1, > UINT8] > +UINT16_V = [0, 0x100, 0x1000, UINT16/4, UINT16/2 - 1, UINT16/2, UINT16/2 + 1, > +UINT16 - 1, UINT16] > UINT32_V = [0, 0x100, 0x1000, 0x1, 0x10, UINT32/4, UINT32/2 - 1, > UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32] > UINT64_V = UINT32_V + [0x100, 0x1000, 0x1, UINT64/4, Seeing some pattern in the vectors, but since the types are very few, we can just construct it like this for now. > @@ -353,3 +355,15 @@ def l2_entry(current): > value = offset + (is_cow << UINT64_M) + \ > (is_compressed << UINT64_M - 1) + is_zero > return value > + > + > +def refcount_table_entry(current): > +"""Fuzz an entry of the refcount table.""" > +constraints = UINT64_V > +return selector(current, constraints) > + > + > +def refcount_block_entry(current): > +"""Fuzz an entry of a refcount block.""" > +constraints = UINT16_V > +return selector(current, constraints) > -- > 1.9.3 > Reviewed-by: Fam Zheng
Re: [Qemu-devel] [RFC 0/4] Adding -cdrom, -hd[abcd] and -drive file=... to Q35
John Snow writes: > Currently, the drive definitions created by drive_new() when using > the -drive file=...[,if=ide] or -cdrom or -hd[abcd] options are not > picked up by the Q35 initialization routine. > > To fix this, we have to add hooks to search for these drives using > something like pc_piix's ide_drive_get and then add them using > something like pci_ide_create_devs. ide_drive_get() isn't pc_piix's, it's a helper function in the IDE core which boards (not just pc_piix) use to find the if=ide drives. It fills in an array of DriveInfo. pci_ide_create_devs() is a helper function in the IDE PCI code which PCI IDE controllers (not just piix3-ide) use to create IDE devices for an array of DriveInfo. > Where it gets slightly wonky is the fact that if=ide is specified > to use two devices per bus, whereas AHCI does not utilize that > same master/slave mechanic. Therefore, many places inside of > blockdev.c where we add and define new drives use incorrect math > for AHCI devices and try to place them on impossible buses. > Notably -hdb and -hdd would become inaccessible. Yes. > To remedy this, I added a new interface type, IF_AHCI. Corresponding > to this change, I modified the default machine properties for Q35 > to use this interface as a default. > > The changes appear to work well, but where I'd like some feedback > is what should happen if people do something like: > > qemu -M q35 -drive if=ide,file=fedora.qcow2 > > The code as presented here is not going to look for or attempt to > connect IDE devices, because it is now looking for /AHCI/ devices. > > At worst, this may break a few existing scripts, but I actually think > that since the if=ide,file=... shorthand never worked to begin with, > the impact might actually be minimal. > > But since the legacy IDE interface of the ICH9 is as of yet unemulated, > the if=ide drives don't have a reasonable place to go yet. I am also > not sure what a reasonable way to handle people specifying BOTH > if=ide and if=ahci drives would be. We've been through IF_AHCI before, more than once, but that was before you got involved :) The problem at hand is that "-drive if=ide" and its sugared forms -hda, -hdb, -cdrom, ... don't work with Q35. You provide a solution for the sugared forms, you leave the problem unsolved for the unsugared form, and you add a new problem: -drive if=ahci doesn't work with i440FX. Progress, sort of. Let's take a step back, and recap previous discussion. There are two defensible point of views, in my opinion. One is that IDE and AHCI should be separate interface types, just like IDE and SCSI are. Attempts to define an if=X drive with a board that doesn't provide a controller for X should fail[*]. Only onboard controllers matter, add-ons plugged in with -device don't. An i440FX board provides only IDE. A Q35 board provides only AHCI, not IDE. If we implement an ich9-ahci legacy mode, and switch it on, then it provides only IDE, not AHCI. Or maybe both, depending on how we do it. The other point of view is that IDE and AHCI are no more different than the different kinds of SCSI HBAs. This is certainly true from a qdev point of view: just like SCSI devices can connect to any SCSI qbus, regardless of the HBA providing it, so can IDE devices connect to any IDE qbus, regardless of the controller providing it. There's a wrinkle: the mapping between index to (bus, unit). This mapping is ABI. The current mapping makes sense for the first generation of controllers: PATA (two devices per bus, thus if_max_devs[IF_IDE] = 2), and 8-bit SCSI (seven per bus, thus if_max_devs[IF_SCSI = 7). The mapping is silly for newer SCSI HBAs. Commit 622b520f tried to make it less silly, but had to be reverted in 27d6bf4 because the silliness was ABI. The mapping is also silly for ich9-ahci. You side-step that silliness only, by adding a new interface type for it. But shouldn't we add a number of SCSI interface types then, too? Where does that end? Can we do better? I think we can, by making this part of the ABI board-specific. The general form of the mapping remains (bus, unit) = (index / N, index % N) but N now depends on board and interface type, not just the latter. If the board connects if=scsi to an lsi53c895a, then N = 7. If the board connects if=ide to an piix3-ide, then N = 2. If the board connects if=ide to an ich9-ahci, then N = 1. I trust you get the idea :) [*] Currently, they're silently ignored with most boards for most X, but I regard that as implementation defect.
Re: [Qemu-devel] [Question] Why doesn't PCIe hotplug work for Q35 machine?
Il 19/08/2014 08:25, Gonglei (Arei) ha scritto: > > 1. Does qemu support ARI Forwarding for PCIe at present? If yes, how to > enable it ? No, not yet. > 2. If not, we should add some check for PCIe root ports and downstream ports, > meanwhile add explaining document. > 3. Those check should add in general code level, both hotplug and coldplug. That would be possible, yes. Paolo
Re: [Qemu-devel] [Question] Why doesn't PCIe hotplug work for Q35 machine?
Hi, > -Original Message- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Tuesday, August 19, 2014 4:06 PM > To: Gonglei (Arei); Marcel Apfelbaum; Michael S. Tsirkin > Cc: qemu-devel@nongnu.org; imamm...@redhat.com; arm...@redhat.com; > Huangweidong (C) > Subject: Re: [Question] Why doesn't PCIe hotplug work for Q35 machine? > > Il 19/08/2014 08:25, Gonglei (Arei) ha scritto: > > > > 1. Does qemu support ARI Forwarding for PCIe at present? If yes, how to > enable it ? > > No, not yet. > > > 2. If not, we should add some check for PCIe root ports and downstream > ports, > > meanwhile add explaining document. > > 3. Those check should add in general code level, both hotplug and coldplug. > > That would be possible, yes. > OK, I got it. Some work in progress for this. Thanks again, Paolo. Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation
On 19 August 2014 08:24, Eric Auger wrote: > Putting size=1 was the only solution I found to use an offset relative > to the parent bus instead of an absolute base address. I would explain > this because, in platform_bus_create_devtree, the function that creates > the "platform bus" node, #address-cells and #size-cells currently are > set to 1. I assume the motivation was that bus size was supposed to be > smaller than 4GB. Then I guess the problem is shifted to the inclusion > of the platform bus in any ARM platform. Ah, I see. Yes, if the containing node is setting addr/size to 1 then 1 is correct, and the limitation then is just the 4GB max. thanks -- PMM
Re: [Qemu-devel] [PATCH V3] spapr: Fix stale HTAB during live migration
* Samuel Mendoza-Jonas (sam...@au1.ibm.com) wrote: > If a guest reboots during a running migration, changes to the > hash page table are not necessarily updated on the destination. > Opening a new file descriptor to the HTAB forces the migration > handler to resend the entire table. Yes I think that's safe. > Signed-off-by: Samuel Mendoza-Jonas > --- > Changes in v3: Pointed out by David, htab_save_iterate could > potentially try to read before htab_fd is open again. > Leave opening the fd to the functions trying to read. > Changes in v2: Forgot check on kvmppc_get_htab_fd return value > hw/ppc/spapr.c | 25 + > include/hw/ppc/spapr.h | 1 + > 2 files changed, 26 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 3a6d26d..5b41318 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -997,6 +997,10 @@ static void spapr_reset_htab(sPAPREnvironment *spapr) > /* Kernel handles htab, we don't need to allocate one */ > spapr->htab_shift = shift; > kvmppc_kern_htab = true; > + > +/* Check if we are overlapping a migration */ > +if (spapr->htab_fd > 0) > +spapr->need_reset = true; > } else { > if (!spapr->htab) { > /* Allocate an htab if we don't yet have one */ > @@ -1156,6 +1160,7 @@ static int htab_save_setup(QEMUFile *f, void *opaque) > } else { > assert(kvm_enabled()); > > +spapr->need_reset = false; > spapr->htab_fd = kvmppc_get_htab_fd(false); > if (spapr->htab_fd < 0) { > fprintf(stderr, "Unable to open fd for reading hash table from > KVM: %s\n", > @@ -1309,6 +1314,16 @@ static int htab_save_iterate(QEMUFile *f, void *opaque) > if (!spapr->htab) { > assert(kvm_enabled()); > > +if (atomic_cmpxchg(&spapr->need_reset, true, false) == true) { > +close(spapr->htab_fd); > +spapr->htab_fd = kvmppc_get_htab_fd(false); > +if (spapr->htab_fd < 0) { > +fprintf(stderr, "Unable to open fd for reading hash table > from KVM: %s\n", > +strerror(errno)); Either perror or error_report() with the strerror would seem better. > +return -1; > +} > +} > + Why not make a little function for this; it seems a bad idea to have two copies of it. Also, add a comment saying why you're reopening it. Dave > rc = kvmppc_save_htab(f, spapr->htab_fd, >MAX_KVM_BUF_SIZE, MAX_ITERATION_NS); > if (rc < 0) { > @@ -1340,6 +1355,16 @@ static int htab_save_complete(QEMUFile *f, void > *opaque) > > assert(kvm_enabled()); > > +if (atomic_cmpxchg(&spapr->need_reset, true, false) == true) { > +close(spapr->htab_fd); > +spapr->htab_fd = kvmppc_get_htab_fd(false); > +if (spapr->htab_fd < 0) { > +fprintf(stderr, "Unable to open fd for reading hash table > from KVM: %s\n", > +strerror(errno)); > +return -1; > +} > +} > + > rc = kvmppc_save_htab(f, spapr->htab_fd, MAX_KVM_BUF_SIZE, -1); > if (rc < 0) { > return rc; > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 0c2e3c5..9ab9827 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -71,6 +71,7 @@ typedef struct sPAPREnvironment { > int htab_save_index; > bool htab_first_pass; > int htab_fd; > +bool need_reset; > > /* state for Dynamic Reconfiguration Connectors */ > sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; > -- > 1.9.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH v8] slirp/misc: Use the GLib memory allocation APIs
Here we don't check the return value of malloc() which may fail. Use the g_new() instead, which will abort the program when there is not enough memory. Also, use g_strdup instead of strdup and remove the unnecessary strdup function. Signed-off-by: zhanghailiang Reviewed-by: Alex Bennée --- slirp/misc.c | 20 +++- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/slirp/misc.c b/slirp/misc.c index b8eb74c..6543dc7 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -54,11 +54,11 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char *exec, } tmp_ptr = *ex_ptr; - *ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list)); + *ex_ptr = g_new(struct ex_list, 1); (*ex_ptr)->ex_fport = port; (*ex_ptr)->ex_addr = addr; (*ex_ptr)->ex_pty = do_pty; - (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : strdup(exec); + (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : g_strdup(exec); (*ex_ptr)->ex_next = tmp_ptr; return 0; } @@ -187,7 +187,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty) bptr++; c = *bptr; *bptr++ = (char)0; - argv[i++] = strdup(curarg); + argv[i++] = g_strdup(curarg); } while (c); argv[i] = NULL; @@ -228,20 +228,6 @@ fork_exec(struct socket *so, const char *ex, int do_pty) } #endif -#ifndef HAVE_STRDUP -char * -strdup(str) - const char *str; -{ - char *bptr; - - bptr = (char *)malloc(strlen(str)+1); - strcpy(bptr, str); - - return bptr; -} -#endif - void slirp_connection_info(Slirp *slirp, Monitor *mon) { const char * const tcpstates[] = { -- 1.7.12.4
[Qemu-devel] [PATCH v3 4/4] block: Drop some superfluous casts from void *
They clutter the code. Unfortunately, I can't figure out how to make Coccinelle drop all of them, so I have to settle for common special cases: @@ type T; T *pt; void *pv; @@ - pt = (T *)pv; + pt = pv; @@ type T; @@ - (T *) (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| g_try_malloc\|g_try_malloc0\|g_try_realloc\| g_try_new\|g_try_new0\|g_try_renew\)(...)) Topped off with minor manual style cleanups. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody --- block/vhdx-log.c| 2 +- block/vvfat.c | 8 hw/ide/microdrive.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index eb5c7a0..6547bec 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, buffer = qemu_blockalign(bs, total_length); memcpy(buffer, &new_hdr, sizeof(new_hdr)); -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); +new_desc = buffer + sizeof(new_hdr); data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE); data_tmp = data; diff --git a/block/vvfat.c b/block/vvfat.c index f877e85..e56f766 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) if(first_cluster == 0 && (is_dotdot || is_dot)) continue; - buffer=(char*)g_malloc(length); + buffer = g_malloc(length); snprintf(buffer,length,"%s/%s",dirname,entry->d_name); if(stat(buffer,&st)<0) { @@ -767,7 +767,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) /* create mapping for this file */ if(!is_dot && !is_dotdot && (S_ISDIR(st.st_mode) || st.st_size)) { - s->current_mapping=(mapping_t*)array_get_next(&(s->mapping)); + s->current_mapping = array_get_next(&(s->mapping)); s->current_mapping->begin=0; s->current_mapping->end=st.st_size; /* @@ -811,12 +811,12 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) } /* reget the mapping, since s->mapping was possibly realloc()ed */ -mapping = (mapping_t*)array_get(&(s->mapping), mapping_index); +mapping = array_get(&(s->mapping), mapping_index); first_cluster += (s->directory.next - mapping->info.dir.first_dir_index) * 0x20 / s->cluster_size; mapping->end = first_cluster; -direntry = (direntry_t*)array_get(&(s->directory), mapping->dir_index); +direntry = array_get(&(s->directory), mapping->dir_index); set_begin_of_direntry(direntry, mapping->begin); return 0; diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index 2d70ddb..15671b8 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -567,7 +567,7 @@ PCMCIACardState *dscm1_init(DriveInfo *dinfo) } md->bus.ifs[0].drive_kind = IDE_CFATA; md->bus.ifs[0].mdata_size = METADATA_SIZE; -md->bus.ifs[0].mdata_storage = (uint8_t *) g_malloc0(METADATA_SIZE); +md->bus.ifs[0].mdata_storage = g_malloc0(METADATA_SIZE); return PCMCIA_CARD(md); } -- 1.9.3
[Qemu-devel] [PATCH v3 1/4] block: Use g_new() & friends where that makes obvious sense
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Patch created with Coccinelle, with two manual changes on top: * Add const to bdrv_iterate_format() to keep the types straight * Convert the allocation in bdrv_drop_intermediate(), which Coccinelle inexplicably misses Coccinelle semantic patch: @@ type T; @@ -g_malloc(sizeof(T)) +g_new(T, 1) @@ type T; @@ -g_try_malloc(sizeof(T)) +g_try_new(T, 1) @@ type T; @@ -g_malloc0(sizeof(T)) +g_new0(T, 1) @@ type T; @@ -g_try_malloc0(sizeof(T)) +g_try_new0(T, 1) @@ type T; expression n; @@ -g_malloc(sizeof(T) * (n)) +g_new(T, n) @@ type T; expression n; @@ -g_try_malloc(sizeof(T) * (n)) +g_try_new(T, n) @@ type T; expression n; @@ -g_malloc0(sizeof(T) * (n)) +g_new0(T, n) @@ type T; expression n; @@ -g_try_malloc0(sizeof(T) * (n)) +g_try_new0(T, n) @@ type T; expression p, n; @@ -g_realloc(p, sizeof(T) * (n)) +g_renew(T, p, n) @@ type T; expression p, n; @@ -g_try_realloc(p, sizeof(T) * (n)) +g_try_renew(T, p, n) Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody --- block-migration.c | 6 +++--- block.c| 14 +++--- block/archipelago.c| 6 +++--- block/gluster.c| 8 block/iscsi.c | 2 +- block/nfs.c| 2 +- block/qcow.c | 2 +- block/qcow2-cluster.c | 2 +- block/qcow2-refcount.c | 8 block/qcow2-snapshot.c | 8 block/raw-posix.c | 2 +- block/rbd.c| 4 ++-- block/sheepdog.c | 4 ++-- block/vdi.c| 2 +- block/vhdx.c | 4 ++-- block/vmdk.c | 7 +++ block/vvfat.c | 2 +- blockdev-nbd.c | 2 +- blockdev.c | 2 +- hw/ide/ahci.c | 2 +- qemu-io-cmds.c | 2 +- qemu-io.c | 2 +- 22 files changed, 46 insertions(+), 47 deletions(-) diff --git a/block-migration.c b/block-migration.c index ba3ed36..3ad31a2 100644 --- a/block-migration.c +++ b/block-migration.c @@ -283,7 +283,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) nr_sectors = total_sectors - cur_sector; } -blk = g_malloc(sizeof(BlkMigBlock)); +blk = g_new(BlkMigBlock, 1); blk->buf = g_malloc(BLOCK_SIZE); blk->bmds = bmds; blk->sector = cur_sector; @@ -354,7 +354,7 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs) return; } -bmds = g_malloc0(sizeof(BlkMigDevState)); +bmds = g_new0(BlkMigDevState, 1); bmds->bs = bs; bmds->bulk_completed = 0; bmds->total_sectors = sectors; @@ -465,7 +465,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, } else { nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK; } -blk = g_malloc(sizeof(BlkMigBlock)); +blk = g_new(BlkMigBlock, 1); blk->buf = g_malloc(BLOCK_SIZE); blk->bmds = bmds; blk->sector = sector; diff --git a/block.c b/block.c index 6fa0201..712f5db 100644 --- a/block.c +++ b/block.c @@ -351,7 +351,7 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp) return NULL; } -bs = g_malloc0(sizeof(BlockDriverState)); +bs = g_new0(BlockDriverState, 1); QLIST_INIT(&bs->dirty_bitmaps); pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); if (device_name[0] != '\0') { @@ -2628,7 +2628,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, * into our deletion queue, until we hit the 'base' */ while (intermediate) { -intermediate_state = g_malloc0(sizeof(BlkIntermediateStates)); +intermediate_state = g_new0(BlkIntermediateStates, 1); intermediate_state->bs = intermediate; QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry); @@ -3755,7 +3755,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name), } if (!found) { -formats = g_realloc(formats, (count + 1) * sizeof(char *)); +formats = g_renew(const char *, formats, count + 1); formats[count++] = drv->format_name; it(opaque, drv->format_name); } @@ -5330,7 +5330,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity, errno = -bitmap_size; return NULL; } -bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); +bitmap = g_new0(BdrvDirtyBitmap, 1); bitmap->bitmap = hbitmap_all
[Qemu-devel] [PATCH v3 0/4] block: Use g_new() & friends more
PATCH 1+2 convert some allocations. While preparing them, I stumbled over dead error handling and some useless casts, which led to PATCH 3+4. I posted a tree-wide version of PATCH 1 some time ago, and was told to split it up. This is the block part, redone from scratch. Other parts available on request. v3: * Manual style cleanups in PATCH 4 [Max, Jeff] * PATCH 1-3 unchanged. v2: * Regenerated with spatch. * Because of that, I decided not to carry Max's R-by forward. Markus Armbruster (4): block: Use g_new() & friends where that makes obvious sense block: Use g_new() & friends to avoid multiplying sizes qemu-io-cmds: g_renew() can't fail, bury dead error handling block: Drop some superfluous casts from void * block-migration.c | 6 +++--- block.c| 14 +++--- block/archipelago.c| 6 +++--- block/bochs.c | 2 +- block/gluster.c| 8 block/iscsi.c | 2 +- block/nfs.c| 2 +- block/parallels.c | 2 +- block/qcow.c | 2 +- block/qcow2-cache.c| 2 +- block/qcow2-cluster.c | 2 +- block/qcow2-refcount.c | 8 block/qcow2-snapshot.c | 8 block/qed-check.c | 3 +-- block/raw-posix.c | 2 +- block/rbd.c| 6 +++--- block/sheepdog.c | 6 +++--- block/vdi.c| 2 +- block/vhdx-log.c | 2 +- block/vhdx.c | 4 ++-- block/vmdk.c | 7 +++ block/vvfat.c | 10 +- blockdev-nbd.c | 2 +- blockdev.c | 2 +- hw/block/nvme.c| 8 hw/ide/ahci.c | 2 +- hw/ide/microdrive.c| 2 +- qemu-io-cmds.c | 21 ++--- qemu-io.c | 2 +- 29 files changed, 67 insertions(+), 78 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v3 2/4] block: Use g_new() & friends to avoid multiplying sizes
g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Perhaps a conversion to g_malloc_n() would be neater in places, but that's merely four years old, and we can't use such newfangled stuff. This commit only touches allocations with size arguments of the form sizeof(T), plus two that use 4 instead of sizeof(uint32_t). We can make the others safe by converting to g_malloc_n() when it becomes available to us in a couple of years. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody --- block/bochs.c | 2 +- block/parallels.c | 2 +- block/qcow2-cache.c | 2 +- block/qed-check.c | 3 +-- block/rbd.c | 2 +- block/sheepdog.c| 2 +- hw/block/nvme.c | 8 qemu-io-cmds.c | 10 +- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/block/bochs.c b/block/bochs.c index 6674b27..199ac2b 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -131,7 +131,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, return -EFBIG; } -s->catalog_bitmap = g_try_malloc(s->catalog_size * 4); +s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size); if (s->catalog_size && s->catalog_bitmap == NULL) { error_setg(errp, "Could not allocate memory for catalog"); return -ENOMEM; diff --git a/block/parallels.c b/block/parallels.c index 1774ab8..2a814f3 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -121,7 +121,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, ret = -EFBIG; goto fail; } -s->catalog_bitmap = g_try_malloc(s->catalog_size * 4); +s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size); if (s->catalog_size && s->catalog_bitmap == NULL) { ret = -ENOMEM; goto fail; diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 5353b44..fe0615a 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -50,7 +50,7 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) c = g_malloc0(sizeof(*c)); c->size = num_tables; -c->entries = g_malloc0(sizeof(*c->entries) * num_tables); +c->entries = g_new0(Qcow2CachedTable, num_tables); for (i = 0; i < c->size; i++) { c->entries[i].table = qemu_try_blockalign(bs->file, s->cluster_size); diff --git a/block/qed-check.c b/block/qed-check.c index 40a882c..36ecd29 100644 --- a/block/qed-check.c +++ b/block/qed-check.c @@ -227,8 +227,7 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix) }; int ret; -check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) * -sizeof(check.used_clusters[0])); +check.used_clusters = g_try_new0(uint32_t, (check.nclusters + 31) / 32); if (check.nclusters && check.used_clusters == NULL) { return -ENOMEM; } diff --git a/block/rbd.c b/block/rbd.c index 3aaf855..ea969e7 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -862,7 +862,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, int max_snaps = RBD_MAX_SNAPS; do { -snaps = g_malloc(sizeof(*snaps) * max_snaps); +snaps = g_new(rbd_snap_info_t, max_snaps); snap_count = rbd_snap_list(s->image, snaps, &max_snaps); if (snap_count <= 0) { g_free(snaps); diff --git a/block/sheepdog.c b/block/sheepdog.c index ba1ef43..12cbd9d 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2357,7 +2357,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) goto out; } -sn_tab = g_malloc0(nr * sizeof(*sn_tab)); +sn_tab = g_new0(QEMUSnapshotInfo, nr); /* calculate a vdi id with hash function */ hval = fnv_64a_buf(s->name, strlen(s->name), FNV1A_64_INIT); diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 5fd8f89..47577ec 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -319,7 +319,7 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr, sq->size = size; sq->cqid = cqid; sq->head = sq->tail = 0; -sq->io_req = g_malloc(sq->size * sizeof(*sq->io_req)); +sq->io_req = g_new(NvmeRequest, sq->size); QTAILQ_INIT(&sq->req_list); QTAILQ_INIT(&sq->out_req_list); @@ -773,9 +773,9 @@ static int nvme_init(PCIDevice *pci_dev) n->reg_size = 1 << qemu_fls(0x1004 + 2 * (n->num_queues + 1) * 4); n->ns_size = bs_size / (uint64_t)n->num_namespaces; -n->namespaces = g_malloc0(sizeof(*n->namespaces)*n->num_namespaces); -n->sq = g_malloc0(sizeof(*n->sq)*n->num_queues); -n->cq = g_malloc0(sizeof(*n->cq)*n->num_queues); +n->namespaces = g_new0(NvmeNamespace, n->num_namespaces); +n->sq = g_new0(NvmeSQueue *, n->num_queues); +n->cq = g_new0(NvmeCQueue *, n->num_queues);
[Qemu-devel] [PULL v2 03/23] linux-user: Fix syscall instruction usermode emulation on X86_64
From: Jincheng Miao Currently syscall instruction is buggy on user mode X86_64, the EIP is updated after do_syscall(), that is too late for clone(). Because clone() will create a thread at the env->EIP (the address of syscall insn), and then child thread enters do_syscall() again, that is not expected. Sometimes it is tragic. User mode syscall insn emulation is not used MSR, so the action should be same to INT 0x80. INT 0x80 will update EIP in do_interrupt(), ditto for syscall() for consistency. Signed-off-by: Jincheng Miao Reviewed-by: Richard Henderson Signed-off-by: Riku Voipio --- linux-user/main.c| 1 - target-i386/seg_helper.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index b453a39..472a16d 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -309,7 +309,6 @@ void cpu_loop(CPUX86State *env) env->regs[8], env->regs[9], 0, 0); -env->eip = env->exception_next_eip; break; #endif case EXCP0B_NOSEG: diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c index 2d970d0..13eefba 100644 --- a/target-i386/seg_helper.c +++ b/target-i386/seg_helper.c @@ -1127,8 +1127,8 @@ static void do_interrupt_user(CPUX86State *env, int intno, int is_int, /* Since we emulate only user space, we cannot do more than exiting the emulation with the suitable exception and error - code */ -if (is_int) { + code. So update EIP for INT 0x80 and EXCP_SYSCALL. */ +if (is_int || intno == EXCP_SYSCALL) { env->eip = next_eip; } } -- 2.0.1
[Qemu-devel] [PULL v2 05/23] linux-user: fix readlink handling with magic exe symlink
From: Mike Frysinger The current code always returns the length of the path when it should be returning the number of bytes it wrote to the output string. Further, readlink is not supposed to append a NUL byte, but the current snprintf logic will always do just that. Even further, if you pass in a length of 0, you're suppoesd to get back an error (EINVAL), but the current logic just returns 0. Further still, if there was an error reading the symlink, we should not go ahead and try to read the target buffer as it is garbage. Simple test for the first two issues: $ cat test.c int main() { char buf[50]; size_t len; for (len = 0; len < 10; ++len) { memset(buf, '!', sizeof(buf)); ssize_t ret = readlink("/proc/self/exe", buf, len); buf[20] = '\0'; printf("readlink(/proc/self/exe, {%s}, %zu) = %zi\n", buf, len, ret); } return 0; } Now compare the output of the native: $ gcc test.c -o /tmp/x $ /tmp/x $ strace /tmp/x With what qemu does: $ armv7a-cros-linux-gnueabi-gcc test.c -o /tmp/x -static $ qemu-arm /tmp/x $ qemu-arm -strace /tmp/x Signed-off-by: Mike Frysinger Signed-off-by: Riku Voipio --- linux-user/syscall.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index fccf9f0..7c108ab 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -6636,11 +6636,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0); if (!p || !p2) { ret = -TARGET_EFAULT; +} else if (!arg3) { +/* Short circuit this for the magic exe check. */ +ret = -TARGET_EINVAL; } else if (is_proc_myself((const char *)p, "exe")) { char real[PATH_MAX], *temp; temp = realpath(exec_path, real); -ret = temp == NULL ? get_errno(-1) : strlen(real) ; -snprintf((char *)p2, arg3, "%s", real); +/* Return value is # of bytes that we wrote to the buffer. */ +if (temp == NULL) { +ret = get_errno(-1); +} else { +/* Don't worry about sign mismatch as earlier mapping + * logic would have thrown a bad address error. */ +ret = MIN(strlen(real), arg3); +/* We cannot NUL terminate the string. */ +memcpy(p2, real, ret); +} } else { ret = get_errno(readlink(path(p), p2, arg3)); } -- 2.0.1
[Qemu-devel] [PATCH v3 3/4] qemu-io-cmds: g_renew() can't fail, bury dead error handling
Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody --- qemu-io-cmds.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index afd8867..b224ede 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -115,22 +115,13 @@ static char **breakline(char *input, int *count) int c = 0; char *p; char **rval = g_new0(char *, 1); -char **tmp; while (rval && (p = qemu_strsep(&input, " ")) != NULL) { if (!*p) { continue; } c++; -tmp = g_renew(char *, rval, (c + 1)); -if (!tmp) { -g_free(rval); -rval = NULL; -c = 0; -break; -} else { -rval = tmp; -} +rval = g_renew(char *, rval, (c + 1)); rval[c - 1] = p; rval[c] = NULL; } -- 1.9.3
[Qemu-devel] [PULL v2 07/23] linux-user: support ioprio_{get, set} syscalls
From: Paul Burton Add support for the ioprio_get & ioprio_set syscalls, allowing their use by target programs. Signed-off-by: Paul Burton Signed-off-by: Riku Voipio --- linux-user/syscall.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 44853d0..f1c182b 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -252,6 +252,12 @@ _syscall2(int, capget, struct __user_cap_header_struct *, header, struct __user_cap_data_struct *, data); _syscall2(int, capset, struct __user_cap_header_struct *, header, struct __user_cap_data_struct *, data); +#if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get) +_syscall2(int, ioprio_get, int, which, int, who) +#endif +#if defined(TARGET_NR_ioprio_set) && defined(__NR_ioprio_set) +_syscall3(int, ioprio_set, int, which, int, who, int, ioprio) +#endif static bitmask_transtbl fcntl_flags_tbl[] = { { TARGET_O_ACCMODE, TARGET_O_WRONLY,O_ACCMODE, O_WRONLY,}, @@ -9592,6 +9598,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; #endif +#if defined(TARGET_NR_ioprio_get) && defined(__NR_ioprio_get) +case TARGET_NR_ioprio_get: +ret = get_errno(ioprio_get(arg1, arg2)); +break; +#endif + +#if defined(TARGET_NR_ioprio_set) && defined(__NR_ioprio_set) +case TARGET_NR_ioprio_set: +ret = get_errno(ioprio_set(arg1, arg2, arg3)); +break; +#endif + default: unimplemented: gemu_log("qemu: Unsupported syscall: %d\n", num); -- 2.0.1
[Qemu-devel] [PULL v2 11/23] linux-user: Dereference Pointer Argument to ipc/semctl Sys Call
From: Tom Musta When the ipc system call is used to wrap a semctl system call, the ptr argument to ipc needs to be dereferenced prior to passing it to the semctl handler. This is because the fourth argument to semctl is a union and not a pointer to a union. Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/syscall.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 239e682..0113250 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3140,9 +3140,15 @@ static abi_long do_ipc(unsigned int call, int first, ret = get_errno(semget(first, second, third)); break; -case IPCOP_semctl: -ret = do_semctl(first, second, third, (union target_semun)(abi_ulong) ptr); +case IPCOP_semctl: { +/* The semun argument to semctl is passed by value, so dereference the + * ptr argument. */ +abi_ulong atptr; +get_user_ual(atptr, (abi_ulong)ptr); +ret = do_semctl(first, second, third, +(union target_semun)(abi_ulong) atptr); break; +} case IPCOP_msgget: ret = get_errno(msgget(first, second)); -- 2.0.1
[Qemu-devel] [PULL v2 00/23] linux-user updates
From: Riku Voipio The same as previous series, except the patch "make binfmt flag O require P" has been dropped. The following changes since commit 142f4ac5d5e024670ef4725e8943702b027e4218: Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-2014-08-15' into staging (2014-08-15 18:44:48 +0100) are available in the git repository at: git://git.linaro.org/people/riku.voipio/qemu.git linux-user-for-upstream for you to fetch changes up to 8349027af60e861fcc401c928eb76f9f418815d7: linux-user: check return value of malloc() (2014-08-18 14:03:29 +0300) Jincheng Miao (1): linux-user: Fix syscall instruction usermode emulation on X86_64 Mike Frysinger (1): linux-user: fix readlink handling with magic exe symlink Mikhail Ilyin (1): linux-user: /proc/self/maps content Paul Burton (1): linux-user: support ioprio_{get, set} syscalls Peter Maydell (1): linux-user: Fix conversion of sigevent argument to timer_create Riku Voipio (4): linux-user: redirect openat calls linux-user: support timerfd_{create, gettime, settime} syscalls linux-user: support {name_to,open_by}_handle_at syscalls linux-user: add setns and unshare Tom Musta (13): linux-user: PPC64 semid_ds Doesnt Include _unused1 and _unused2 linux-user: Dereference Pointer Argument to ipc/semctl Sys Call linux-user: Properly Handle semun Structure In Cross-Endian Situations linux-user: Make ipc syscall's third argument an abi_long linux-user: Conditionally Pass Attribute Pointer to mq_open() linux-user: Detect Negative Message Sizes in msgsnd System Call linux-user: Handle NULL sched_param argument to sched_* linux-user: Detect fault in sched_rr_get_interval linux-user: Move get_ppc64_abi linux-user: Minimum Sig Handler Stack Size for PPC64 ELF V2 linux-user: clock_nanosleep errno Handling on PPC linux-user: Support target-to-host translation of mlockall argument linux-user: writev Partial Writes zhanghailiang (1): linux-user: check return value of malloc() configure | 37 + include/exec/cpu-all.h | 2 + linux-user/aarch64/syscall.h| 3 + linux-user/alpha/syscall.h | 3 + linux-user/arm/syscall.h| 4 + linux-user/cris/syscall.h | 3 + linux-user/elfload.c| 9 -- linux-user/i386/syscall.h | 3 + linux-user/m68k/syscall.h | 4 + linux-user/main.c | 1 - linux-user/microblaze/syscall.h | 3 + linux-user/mips/syscall.h | 3 + linux-user/mips64/syscall.h | 3 + linux-user/openrisc/syscall.h | 4 + linux-user/ppc/syscall.h| 4 + linux-user/ppc/target_cpu.h | 10 ++ linux-user/s390x/syscall.h | 3 + linux-user/sh4/syscall.h| 4 + linux-user/signal.c | 12 +- linux-user/sparc/syscall.h | 3 + linux-user/sparc64/syscall.h| 3 + linux-user/strace.c | 30 linux-user/strace.list | 18 +++ linux-user/syscall.c| 347 ++-- linux-user/unicore32/syscall.h | 4 + linux-user/x86_64/syscall.h | 3 + target-i386/seg_helper.c| 4 +- 27 files changed, 462 insertions(+), 65 deletions(-) -- 2.0.1
[Qemu-devel] [PULL v2 08/23] linux-user: support {name_to, open_by}_handle_at syscalls
From: Riku Voipio Implement support for the name_to_handle_at and open_by_handle_at syscalls, allowing their use by the target program. Modified by Riku - move syscalls to functions and put behind the already existing CONFIG_OPEN_BY_HANDLE to avoid build failure with old glibc's. Signed-off-by: Paul Burton Signed-off-by: Riku Voipio --- linux-user/strace.c| 30 ++ linux-user/strace.list | 6 + linux-user/syscall.c | 70 ++ 3 files changed, 106 insertions(+) diff --git a/linux-user/strace.c b/linux-user/strace.c index ea6c1d2..c20ddf1 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -1552,6 +1552,36 @@ print_kill(const struct syscallname *name, } #endif +#ifdef TARGET_NR_name_to_handle_at +static void +print_name_to_handle_at(const struct syscallname *name, +abi_long arg0, abi_long arg1, abi_long arg2, +abi_long arg3, abi_long arg4, abi_long arg5) +{ +print_syscall_prologue(name); +print_at_dirfd(arg0, 0); +print_string(arg1, 0); +print_pointer(arg2, 0); +print_pointer(arg3, 0); +print_raw_param("0x%x", arg4, 1); +print_syscall_epilogue(name); +} +#endif + +#ifdef TARGET_NR_open_by_handle_at +static void +print_open_by_handle_at(const struct syscallname *name, +abi_long arg0, abi_long arg1, abi_long arg2, +abi_long arg3, abi_long arg4, abi_long arg5) +{ +print_syscall_prologue(name); +print_raw_param("%d", arg0, 0); +print_pointer(arg2, 0); +print_open_flags(arg3, 1); +print_syscall_epilogue(name); +} +#endif + /* * An array of all of the syscalls we know about */ diff --git a/linux-user/strace.list b/linux-user/strace.list index 8de972a..147f579 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -582,6 +582,9 @@ #ifdef TARGET_NR_munmap { TARGET_NR_munmap, "munmap" , NULL, print_munmap, NULL }, #endif +#ifdef TARGET_NR_name_to_handle_at +{ TARGET_NR_name_to_handle_at, "name_to_handle_at" , NULL, print_name_to_handle_at, NULL }, +#endif #ifdef TARGET_NR_nanosleep { TARGET_NR_nanosleep, "nanosleep" , NULL, NULL, NULL }, #endif @@ -624,6 +627,9 @@ #ifdef TARGET_NR_openat { TARGET_NR_openat, "openat" , NULL, print_openat, NULL }, #endif +#ifdef TARGET_NR_open_by_handle_at +{ TARGET_NR_open_by_handle_at, "open_by_handle_at" , NULL, print_open_by_handle_at, NULL }, +#endif #ifdef TARGET_NR_osf_adjtime { TARGET_NR_osf_adjtime, "osf_adjtime" , NULL, NULL, NULL }, #endif diff --git a/linux-user/syscall.c b/linux-user/syscall.c index f1c182b..74c5d49 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5362,6 +5362,66 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags, return get_errno(sys_openat(dirfd, path(pathname), flags, mode)); } +#if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE) +static abi_long do_name_to_handle_at(abi_long arg1, abi_long arg2, +abi_long arg3, abi_long arg4, abi_long arg5) +{ +struct file_handle *fh; +uint32_t sz; +int mount_id; +abi_long ret; +char *p; + +if (get_user_u32(sz, arg3)) { +return -TARGET_EFAULT; +} + +p = lock_user_string(arg2); +if (!p) { +return -TARGET_EFAULT; +} + +fh = lock_user(VERIFY_WRITE, arg3, sizeof(*fh) + sz, 1); +if (!fh) { +unlock_user(p, arg2, 0); +return -TARGET_EFAULT; +} + +ret = get_errno(name_to_handle_at(arg1, path(p), fh, &mount_id, arg5)); +unlock_user(p, arg2, 0); +unlock_user(p, arg3, sizeof(*fh) + sz); + +if (put_user_s32(mount_id, arg4)) { +return -TARGET_EFAULT; +} +return ret; + +} +#endif +#if defined(TARGET_NR_open_by_handle_at) && defined(CONFIG_OPEN_BY_HANDLE) +static abi_long do_open_by_handle_at(abi_long arg1, abi_long arg2, abi_long arg3) +{ +struct file_handle *fh; +uint32_t sz; +abi_long ret; +char *p; + +if (get_user_u32(sz, arg2)) { +return -TARGET_EFAULT; +} + +fh = lock_user(VERIFY_WRITE, arg2, sizeof(*fh) + sz, 1); +if (!fh) { +return -TARGET_EFAULT; +} + +ret = get_errno(open_by_handle_at(arg1, fh, +target_to_host_bitmask(arg3, fcntl_flags_tbl))); + +unlock_user(p, arg2, sizeof(*fh) + sz); +return ret; +} +#endif /* do_syscall() should always have a single exit point at the end so that actions, such as logging of syscall results, can be performed. @@ -5448,6 +5508,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, arg4)); unlock_user(p, arg2, 0); break; +#if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE) +case TARGET_NR_name_to_handle_at: +ret = do_name_to_handle_at(arg1, arg2, arg3, arg4, arg5); +break; +#endif +#if defined(TARGET_NR_open_by_handle_at) && defined(CONFIG_OPEN_BY_HANDLE) +case TARGET_NR_open_by_handle_at: +ret = do_ope
[Qemu-devel] [PULL v2 01/23] linux-user: /proc/self/maps content
From: Mikhail Ilyin Build /proc/self/maps doing a match against guest memory translation table. Output only that map records which are valid for guest memory layout. Signed-off-by: Mikhail Ilyin Signed-off-by: Riku Voipio --- include/exec/cpu-all.h | 2 ++ linux-user/syscall.c | 25 ++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index f91581f..f9d132f 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -198,6 +198,8 @@ extern unsigned long reserved_va; #define RESERVED_VA 0ul #endif +#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \ +(1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1) #endif /* page related stuff */ diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a50229d..c8c2b4c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd) static int open_self_maps(void *cpu_env, int fd) { -#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env); TaskState *ts = cpu->opaque; -#endif FILE *fp; char *line = NULL; size_t len = 0; @@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd) if ((fields < 10) || (fields > 11)) { continue; } -if (!strncmp(path, "[stack]", 7)) { -continue; -} -if (h2g_valid(min) && h2g_valid(max)) { +if (h2g_valid(min)) { +int flags = page_get_flags(h2g(min)); +max = h2g_valid(max - 1) ? max : (uintptr_t)g2h(GUEST_ADDR_MAX); +if (page_check_range(h2g(min), max - min, flags) == -1) { +continue; +} +if (h2g(min) == ts->info->stack_limit) { +pstrcpy(path, sizeof(path), " [stack]"); +} dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n", -h2g(min), h2g(max), flag_r, flag_w, +h2g(min), h2g(max - 1) + 1, flag_r, flag_w, flag_x, flag_p, offset, dev_maj, dev_min, inode, path[0] ? " " : "", path); } @@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd) free(line); fclose(fp); -#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) -dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0 [stack]\n", -(unsigned long long)ts->info->stack_limit, -(unsigned long long)(ts->info->start_stack + - (TARGET_PAGE_SIZE - 1)) & TARGET_PAGE_MASK, -(unsigned long long)0); -#endif - return 0; } -- 2.0.1
[Qemu-devel] [PULL v2 02/23] linux-user: redirect openat calls
From: Riku Voipio While Mikhail fixed /proc/self/maps, it was noticed openat calls are not redirected currently. Some archs don't have open at all, so openat needs to be redirected. Fix this by consolidating open/openat code to do_openat - open is implemented using openat(AT_FDCWD, ... ), which according to open(2) man page is identical. Since all targets now have openat, remove the ifdef around sys_openat and openat: case in do_syscall. Cc: Mikhail Ilin Signed-off-by: Riku Voipio --- linux-user/syscall.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index c8c2b4c..dd77673 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -294,7 +294,6 @@ static int sys_getcwd1(char *buf, size_t size) return strlen(buf)+1; } -#ifdef TARGET_NR_openat static int sys_openat(int dirfd, const char *pathname, int flags, mode_t mode) { /* @@ -306,7 +305,6 @@ static int sys_openat(int dirfd, const char *pathname, int flags, mode_t mode) } return (openat(dirfd, pathname, flags)); } -#endif #ifdef TARGET_NR_utimensat #ifdef CONFIG_UTIMENSAT @@ -5274,7 +5272,7 @@ static int open_net_route(void *cpu_env, int fd) } #endif -static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) +static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags, mode_t mode) { struct fake_open { const char *filename; @@ -5295,7 +5293,7 @@ static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) if (is_proc_myself(pathname, "exe")) { int execfd = qemu_getauxval(AT_EXECFD); -return execfd ? execfd : get_errno(open(exec_path, flags, mode)); +return execfd ? execfd : get_errno(sys_openat(dirfd, exec_path, flags, mode)); } for (fake_open = fakes; fake_open->filename; fake_open++) { @@ -5329,7 +5327,7 @@ static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) return fd; } -return get_errno(open(path(pathname), flags, mode)); +return get_errno(sys_openat(dirfd, path(pathname), flags, mode)); } /* do_syscall() should always have a single exit point at the end so @@ -5404,22 +5402,19 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, case TARGET_NR_open: if (!(p = lock_user_string(arg1))) goto efault; -ret = get_errno(do_open(cpu_env, p, -target_to_host_bitmask(arg2, fcntl_flags_tbl), -arg3)); +ret = get_errno(do_openat(cpu_env, AT_FDCWD, p, + target_to_host_bitmask(arg2, fcntl_flags_tbl), + arg3)); unlock_user(p, arg1, 0); break; -#if defined(TARGET_NR_openat) && defined(__NR_openat) case TARGET_NR_openat: if (!(p = lock_user_string(arg2))) goto efault; -ret = get_errno(sys_openat(arg1, - path(p), - target_to_host_bitmask(arg3, fcntl_flags_tbl), - arg4)); +ret = get_errno(do_openat(cpu_env, arg1, p, + target_to_host_bitmask(arg3, fcntl_flags_tbl), + arg4)); unlock_user(p, arg2, 0); break; -#endif case TARGET_NR_close: ret = get_errno(close(arg1)); break; -- 2.0.1
[Qemu-devel] [PULL v2 12/23] linux-user: Properly Handle semun Structure In Cross-Endian Situations
From: Tom Musta The semun union used in the semctl system call contains both an int (val) and pointers. In cross-endian situations on 64 bit targets, the value passed to semctl is an 8 byte (abi_long) value and thus does not have the 4-byte val field in the correct location. In order to rectify this, the other half of the union must be accessed. This is achieved in code by performing a byte swap on the entire 8 byte union, followed by a 4-byte swap of the first half. Also, eliminate an extraneous (dead) line of code that sets target_su.val in the IPC_SET/IPC_GET case. Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/syscall.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 0113250..ba9dfc5 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -2652,9 +2652,18 @@ static inline abi_long do_semctl(int semid, int semnum, int cmd, switch( cmd ) { case GETVAL: case SETVAL: -arg.val = tswap32(target_su.val); +/* In 64 bit cross-endian situations, we will erroneously pick up + * the wrong half of the union for the "val" element. To rectify + * this, the entire 8-byte structure is byteswapped, followed by +* a swap of the 4 byte val field. In other cases, the data is +* already in proper host byte order. */ + if (sizeof(target_su.val) != (sizeof(target_su.buf))) { + target_su.buf = tswapal(target_su.buf); + arg.val = tswap32(target_su.val); + } else { + arg.val = target_su.val; + } ret = get_errno(semctl(semid, semnum, cmd, arg)); -target_su.val = tswap32(arg.val); break; case GETALL: case SETALL: -- 2.0.1
[Qemu-devel] [PULL v2 09/23] linux-user: add setns and unshare
From: Riku Voipio Add support for the setns and unshare syscalls, trivially passed through to the host. Based on patches by Paul Burton, added configure check. Signed-off-by: Paul Burton Signed-off-by: Riku Voipio --- configure | 20 linux-user/strace.list | 3 +++ linux-user/syscall.c | 11 +++ 3 files changed, 34 insertions(+) diff --git a/configure b/configure index 3d0ab1b..c4e47e1 100755 --- a/configure +++ b/configure @@ -3470,6 +3470,23 @@ if compile_prog "" "" ; then timerfd=yes fi +# check for setns and unshare support +setns=no +cat > $TMPC << EOF +#include + +int main(void) +{ +int ret; +ret = setns(0, 0); +ret = unshare(0); +return ret; +} +EOF +if compile_prog "" "" ; then + setns=yes +fi + # Check if tools are available to build documentation. if test "$docs" != "no" ; then if has makeinfo && has pod2man; then @@ -4541,6 +4558,9 @@ fi if test "$timerfd" = "yes" ; then echo "CONFIG_TIMERFD=y" >> $config_host_mak fi +if test "$setns" = "yes" ; then + echo "CONFIG_SETNS=y" >> $config_host_mak +fi if test "$inotify" = "yes" ; then echo "CONFIG_INOTIFY=y" >> $config_host_mak fi diff --git a/linux-user/strace.list b/linux-user/strace.list index 147f579..d5b8033 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -1191,6 +1191,9 @@ #ifdef TARGET_NR_set_mempolicy { TARGET_NR_set_mempolicy, "set_mempolicy" , NULL, NULL, NULL }, #endif +#ifdef TARGET_NR_setns +{ TARGET_NR_setns, "setns" , NULL, NULL, NULL }, +#endif #ifdef TARGET_NR_setpgid { TARGET_NR_setpgid, "setpgid" , NULL, NULL, NULL }, #endif diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 74c5d49..e5c3ebb 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9680,6 +9680,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; #endif +#if defined(TARGET_NR_setns) && defined(CONFIG_SETNS) +case TARGET_NR_setns: +ret = get_errno(setns(arg1, arg2)); +break; +#endif +#if defined(TARGET_NR_unshare) && defined(CONFIG_SETNS) +case TARGET_NR_unshare: +ret = get_errno(unshare(arg1)); +break; +#endif + default: unimplemented: gemu_log("qemu: Unsupported syscall: %d\n", num); -- 2.0.1
[Qemu-devel] [PULL v2 06/23] linux-user: support timerfd_{create, gettime, settime} syscalls
From: Riku Voipio Adds support for the timerfd_create, timerfd_gettime & timerfd_settime syscalls, allowing use of timerfds by target programs. v2: By Riku - added configure check for timerfd and ifdefs for benefit of old distributions like RHEL5. Signed-off-by: Paul Burton Signed-off-by: Riku Voipio --- configure | 17 + linux-user/strace.list | 9 + linux-user/syscall.c | 45 + 3 files changed, 71 insertions(+) diff --git a/configure b/configure index 283c71c..3d0ab1b 100755 --- a/configure +++ b/configure @@ -3456,6 +3456,20 @@ if compile_prog "" "" ; then sendfile=yes fi +# check for timerfd support (glibc 2.8 and newer) +timerfd=no +cat > $TMPC << EOF +#include + +int main(void) +{ +return(timerfd_create(CLOCK_REALTIME, 0)); +} +EOF +if compile_prog "" "" ; then + timerfd=yes +fi + # Check if tools are available to build documentation. if test "$docs" != "no" ; then if has makeinfo && has pod2man; then @@ -4524,6 +4538,9 @@ fi if test "$sendfile" = "yes" ; then echo "CONFIG_SENDFILE=y" >> $config_host_mak fi +if test "$timerfd" = "yes" ; then + echo "CONFIG_TIMERFD=y" >> $config_host_mak +fi if test "$inotify" = "yes" ; then echo "CONFIG_INOTIFY=y" >> $config_host_mak fi diff --git a/linux-user/strace.list b/linux-user/strace.list index fcb258d..8de972a 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -1404,6 +1404,15 @@ #ifdef TARGET_NR_timer_settime { TARGET_NR_timer_settime, "timer_settime" , NULL, NULL, NULL }, #endif +#ifdef TARGET_NR_timerfd_create +{ TARGET_NR_timerfd_create, "timerfd_create" , NULL, NULL, NULL }, +#endif +#ifdef TARGET_NR_timerfd_gettime +{ TARGET_NR_timerfd_gettime, "timerfd_gettime" , NULL, NULL, NULL }, +#endif +#ifdef TARGET_NR_timerfd_settime +{ TARGET_NR_timerfd_settime, "timerfd_settime" , NULL, NULL, NULL }, +#endif #ifdef TARGET_NR_times { TARGET_NR_times, "times" , NULL, NULL, NULL }, #endif diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 7c108ab..44853d0 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -58,6 +58,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include #include #include +#include #include #include //#include @@ -9547,6 +9548,50 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } #endif +#if defined(TARGET_NR_timerfd_create) && defined(CONFIG_TIMERFD) +case TARGET_NR_timerfd_create: +ret = get_errno(timerfd_create(arg1, +target_to_host_bitmask(arg2, fcntl_flags_tbl))); +break; +#endif + +#if defined(TARGET_NR_timerfd_gettime) && defined(CONFIG_TIMERFD) +case TARGET_NR_timerfd_gettime: +{ +struct itimerspec its_curr; + +ret = get_errno(timerfd_gettime(arg1, &its_curr)); + +if (arg2 && host_to_target_itimerspec(arg2, &its_curr)) { +goto efault; +} +} +break; +#endif + +#if defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD) +case TARGET_NR_timerfd_settime: +{ +struct itimerspec its_new, its_old, *p_new; + +if (arg3) { +if (target_to_host_itimerspec(&its_new, arg3)) { +goto efault; +} +p_new = &its_new; +} else { +p_new = NULL; +} + +ret = get_errno(timerfd_settime(arg1, arg2, p_new, &its_old)); + +if (arg4 && host_to_target_itimerspec(arg4, &its_old)) { +goto efault; +} +} +break; +#endif + default: unimplemented: gemu_log("qemu: Unsupported syscall: %d\n", num); -- 2.0.1
[Qemu-devel] [PULL v2 13/23] linux-user: Make ipc syscall's third argument an abi_long
From: Tom Musta For those target ABIs that use the ipc system call (e.g. POWER), the third argument is used in the shmat path as a pointer. It therefore must be declared as an abi_long (versus int) so that the address bits are not lost in truncation. In fact, all arguments to do_ipc should be declared as abit_long. In fact, it makes more sense for all of the arguments to be declaried as abi_long (except call). Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/syscall.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ba9dfc5..db40829 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3130,8 +3130,8 @@ static inline abi_long do_shmdt(abi_ulong shmaddr) #ifdef TARGET_NR_ipc /* ??? This only works with linear mappings. */ /* do_ipc() must return target values and target errnos. */ -static abi_long do_ipc(unsigned int call, int first, - int second, int third, +static abi_long do_ipc(unsigned int call, abi_long first, + abi_long second, abi_long third, abi_long ptr, abi_long fifth) { int version; @@ -3153,9 +3153,9 @@ static abi_long do_ipc(unsigned int call, int first, /* The semun argument to semctl is passed by value, so dereference the * ptr argument. */ abi_ulong atptr; -get_user_ual(atptr, (abi_ulong)ptr); +get_user_ual(atptr, ptr); ret = do_semctl(first, second, third, -(union target_semun)(abi_ulong) atptr); +(union target_semun) atptr); break; } -- 2.0.1
[Qemu-devel] [PULL v2 16/23] linux-user: Handle NULL sched_param argument to sched_*
From: Tom Musta The sched_getparam, sched_setparam and sched_setscheduler system calls take a pointer argument to a sched_param structure. When this pointer is null, errno should be set to EINVAL. Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/syscall.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index d223cff..a0436da 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7806,6 +7806,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, struct sched_param *target_schp; struct sched_param schp; +if (arg2 == 0) { +return -TARGET_EINVAL; +} if (!lock_user_struct(VERIFY_READ, target_schp, arg2, 1)) goto efault; schp.sched_priority = tswap32(target_schp->sched_priority); @@ -7817,6 +7820,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct sched_param *target_schp; struct sched_param schp; + +if (arg2 == 0) { +return -TARGET_EINVAL; +} ret = get_errno(sched_getparam(arg1, &schp)); if (!is_error(ret)) { if (!lock_user_struct(VERIFY_WRITE, target_schp, arg2, 0)) @@ -7830,6 +7837,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct sched_param *target_schp; struct sched_param schp; +if (arg3 == 0) { +return -TARGET_EINVAL; +} if (!lock_user_struct(VERIFY_READ, target_schp, arg3, 1)) goto efault; schp.sched_priority = tswap32(target_schp->sched_priority); -- 2.0.1
[Qemu-devel] [PULL v2 10/23] linux-user: PPC64 semid_ds Doesnt Include _unused1 and _unused2
From: Tom Musta The 64 bit PowerPC platforms eliminate the _unused1 and _unused2 elements of the semid_ds structure from . So eliminate these from the target_semid_ds structure. Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/syscall.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e5c3ebb..239e682 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -2424,9 +2424,13 @@ struct target_semid_ds { struct target_ipc_perm sem_perm; abi_ulong sem_otime; +#if !defined(TARGET_PPC64) abi_ulong __unused1; +#endif abi_ulong sem_ctime; +#if !defined(TARGET_PPC64) abi_ulong __unused2; +#endif abi_ulong sem_nsems; abi_ulong __unused3; abi_ulong __unused4; -- 2.0.1
[Qemu-devel] [PULL v2 15/23] linux-user: Detect Negative Message Sizes in msgsnd System Call
From: Tom Musta The msgsnd system call takes an argument that describes the message size (msgsz) and is of type size_t. The system call should set errno to EINVAL in the event that a negative message size is passed. Signed-off-by: Tom Musta Reviewed-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index c9a9d3d..d223cff 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -2879,12 +2879,16 @@ struct target_msgbuf { }; static inline abi_long do_msgsnd(int msqid, abi_long msgp, - unsigned int msgsz, int msgflg) + ssize_t msgsz, int msgflg) { struct target_msgbuf *target_mb; struct msgbuf *host_mb; abi_long ret = 0; +if (msgsz < 0) { +return -TARGET_EINVAL; +} + if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0)) return -TARGET_EFAULT; host_mb = malloc(msgsz+sizeof(long)); -- 2.0.1
[Qemu-devel] [PULL v2 20/23] linux-user: clock_nanosleep errno Handling on PPC
From: Tom Musta The clock_nanosleep syscall is unusual in that it returns positive numbers in error handling situations, versus returning -1 and setting errno, or returning a negative errno value. On POWER, the kernel will set the SO bit of CR0 to indicate failure in a syscall. QEMU has generic handling to do this for syscalls with standard return values. Add special case code for clock_nanosleep to handle CR0 properly. Signed-off-by: Tom Musta Reviewed-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 8 1 file changed, 8 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a24356d..e4be32c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9103,6 +9103,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = get_errno(clock_nanosleep(arg1, arg2, &ts, arg4 ? &ts : NULL)); if (arg4) host_to_target_timespec(arg4, &ts); + +#if defined(TARGET_PPC) +/* clock_nanosleep is odd in that it returns positive errno values. + * On PPC, CR0 bit 3 should be set in such a situation. */ +if (ret) { +((CPUPPCState *)cpu_env)->crf[0] |= 1; +} +#endif break; } #endif -- 2.0.1
[Qemu-devel] [PULL v2 04/23] linux-user: Fix conversion of sigevent argument to timer_create
From: Peter Maydell There were a number of bugs in the conversion of the sigevent argument to timer_create from target to host format: * signal number not converted from target to host * thread ID not copied across * sigev_value not copied across * we never unlocked the struct when we were done Between them, these problems meant that SIGEV_THREAD_ID timers (and the glibc-implemented SIGEV_THREAD timers which depend on them) didn't work. Fix these problems and clean up the code a little by pulling the struct conversion out into its own function, in line with how we convert various other structs. This allows the test program in bug LP:1042388 to run. Signed-off-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 38 ++ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index dd77673..fccf9f0 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4912,6 +4912,32 @@ static inline abi_long host_to_target_itimerspec(abi_ulong target_addr, return 0; } +static inline abi_long target_to_host_sigevent(struct sigevent *host_sevp, + abi_ulong target_addr) +{ +struct target_sigevent *target_sevp; + +if (!lock_user_struct(VERIFY_READ, target_sevp, target_addr, 1)) { +return -TARGET_EFAULT; +} + +/* This union is awkward on 64 bit systems because it has a 32 bit + * integer and a pointer in it; we follow the conversion approach + * used for handling sigval types in signal.c so the guest should get + * the correct value back even if we did a 64 bit byteswap and it's + * using the 32 bit integer. + */ +host_sevp->sigev_value.sival_ptr = +(void *)(uintptr_t)tswapal(target_sevp->sigev_value.sival_ptr); +host_sevp->sigev_signo = +target_to_host_signal(tswap32(target_sevp->sigev_signo)); +host_sevp->sigev_notify = tswap32(target_sevp->sigev_notify); +host_sevp->_sigev_un._tid = tswap32(target_sevp->_sigev_un._tid); + +unlock_user_struct(target_sevp, target_addr, 1); +return 0; +} + #if defined(TARGET_NR_stat64) || defined(TARGET_NR_newfstatat) static inline abi_long host_to_target_stat64(void *cpu_env, abi_ulong target_addr, @@ -9403,7 +9429,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */ struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL; -struct target_sigevent *ptarget_sevp; struct target_timer_t *ptarget_timer; int clkid = arg1; @@ -9415,14 +9440,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, timer_t *phtimer = g_posix_timers + timer_index; if (arg2) { -if (!lock_user_struct(VERIFY_READ, ptarget_sevp, arg2, 1)) { -goto efault; -} - -host_sevp.sigev_signo = tswap32(ptarget_sevp->sigev_signo); -host_sevp.sigev_notify = tswap32(ptarget_sevp->sigev_notify); - phost_sevp = &host_sevp; +ret = target_to_host_sigevent(phost_sevp, arg2); +if (ret != 0) { +break; +} } ret = get_errno(timer_create(clkid, phost_sevp, phtimer)); -- 2.0.1
[Qemu-devel] [PULL v2 23/23] linux-user: check return value of malloc()
From: zhanghailiang Signed-off-by: zhanghailiang Acked-by: Riku Voipio Signed-off-by: Riku Voipio --- linux-user/syscall.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 3b1beff..3aaed81 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -2904,6 +2904,10 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp, if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0)) return -TARGET_EFAULT; host_mb = malloc(msgsz+sizeof(long)); +if (!host_mb) { +unlock_user_struct(target_mb, msgp, 0); +return -TARGET_ENOMEM; +} host_mb->mtype = (abi_long) tswapal(target_mb->mtype); memcpy(host_mb->mtext, target_mb->mtext, msgsz); ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg)); -- 2.0.1
[Qemu-devel] [PULL v2 22/23] linux-user: writev Partial Writes
From: Tom Musta Although not technically not required by POSIX, the writev system call will typically write out its buffers individually. That is, if the first buffer is written successfully, but the second buffer pointer is invalid, then the first chuck will be written and its size is returned. Signed-off-by: Tom Musta Reviewed-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index c4f6454..3b1beff 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1803,6 +1803,7 @@ static struct iovec *lock_iovec(int type, abi_ulong target_addr, abi_ulong total_len, max_len; int i; int err = 0; +bool bad_address = false; if (count == 0) { errno = 0; @@ -1843,9 +1844,20 @@ static struct iovec *lock_iovec(int type, abi_ulong target_addr, vec[i].iov_base = 0; } else { vec[i].iov_base = lock_user(type, base, len, copy); +/* If the first buffer pointer is bad, this is a fault. But + * subsequent bad buffers will result in a partial write; this + * is realized by filling the vector with null pointers and + * zero lengths. */ if (!vec[i].iov_base) { -err = EFAULT; -goto fail; +if (i == 0) { +err = EFAULT; +goto fail; +} else { +bad_address = true; +} +} +if (bad_address) { +len = 0; } if (len > max_len - total_len) { len = max_len - total_len; -- 2.0.1
[Qemu-devel] [PULL v2 17/23] linux-user: Detect fault in sched_rr_get_interval
From: Tom Musta Properly detect a fault when attempting to store into an invalid struct timespec pointer. Signed-off-by: Tom Musta Reviewed-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a0436da..a24356d 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7864,7 +7864,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, struct timespec ts; ret = get_errno(sched_rr_get_interval(arg1, &ts)); if (!is_error(ret)) { -host_to_target_timespec(arg2, &ts); +ret = host_to_target_timespec(arg2, &ts); } } break; -- 2.0.1
[Qemu-devel] [PULL v2 14/23] linux-user: Conditionally Pass Attribute Pointer to mq_open()
From: Tom Musta The mq_open system call takes an optional struct mq_attr pointer argument in the fourth position. This pointer is used when O_CREAT is specified in the flags (second) argument. It may be NULL, in which case the queue is created with implementation defined attributes. Change the code to properly handle the case when NULL is passed in the arg4 position. Signed-off-by: Tom Musta Reviewed-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index db40829..c9a9d3d 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9185,12 +9185,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open) case TARGET_NR_mq_open: { -struct mq_attr posix_mq_attr; +struct mq_attr posix_mq_attr, *attrp; p = lock_user_string(arg1 - 1); -if (arg4 != 0) +if (arg4 != 0) { copy_from_user_mq_attr (&posix_mq_attr, arg4); -ret = get_errno(mq_open(p, arg2, arg3, &posix_mq_attr)); +attrp = &posix_mq_attr; +} else { +attrp = 0; +} +ret = get_errno(mq_open(p, arg2, arg3, attrp)); unlock_user (p, arg1, 0); } break; -- 2.0.1
[Qemu-devel] [PULL v2 18/23] linux-user: Move get_ppc64_abi
From: Tom Musta The get_ppc64_abi is used to determine the ELF ABI (i.e. V1 or V2). This routine is currently implemented in the linux-user/elfload.c file but is useful in other scenarios. Move the routine to a more generally available location (linux-user/ppc/target_cpu.h). Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/elfload.c| 9 - linux-user/ppc/target_cpu.h | 10 ++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 60777fe..bea803b 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -824,8 +824,6 @@ static uint32_t get_elf_hwcap2(void) NEW_AUX_ENT(AT_IGNOREPPC, AT_IGNOREPPC);\ } while (0) -static inline uint32_t get_ppc64_abi(struct image_info *infop); - static inline void init_thread(struct target_pt_regs *_regs, struct image_info *infop) { _regs->gpr[1] = infop->start_stack; @@ -1205,13 +1203,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i #include "elf.h" -#ifdef TARGET_PPC -static inline uint32_t get_ppc64_abi(struct image_info *infop) -{ - return infop->elf_flags & EF_PPC64_ABI; -} -#endif - struct exec { unsigned int a_info; /* Use macros N_MAGIC, etc for access */ diff --git a/linux-user/ppc/target_cpu.h b/linux-user/ppc/target_cpu.h index 9cc0c3b..26f4ba2 100644 --- a/linux-user/ppc/target_cpu.h +++ b/linux-user/ppc/target_cpu.h @@ -38,4 +38,14 @@ static inline void cpu_set_tls(CPUPPCState *env, target_ulong newtls) #endif } +#ifndef EF_PPC64_ABI +#define EF_PPC64_ABI 0x3 +#endif + +static inline uint32_t get_ppc64_abi(struct image_info *infop) +{ + return infop->elf_flags & EF_PPC64_ABI; +} + + #endif -- 2.0.1
Re: [Qemu-devel] Microcheckpointing: Memory-VCPU / Disk State consistency
Hi, I have tried to find more information on how to use drive-mirror besides what is available on the wiki. This was not very satisfactory... This may sound naive but are there some code examples in "c" or any other language, documentation of any kind, blog entries (developer), presentation videos or any other source of information to get started? Walid > Am 14.08.2014 um 12:58 schrieb "Dr. David Alan Gilbert" : > > cc'ing in a couple of the COLOers. > > * Michael R. Hines (mrhi...@linux.vnet.ibm.com) wrote: >>> On 08/13/2014 10:03 PM, Walid Nouri wrote: >>> >>> While looking to find some ideas for approaches to replicating block >>> devices I have read the paper about the Remus implementation. I think MC >>> can take a similar approach for local disk. >> >> I agree. >> >>> Here are the main facts that I have understood: >>> >>> Local disk contents is viewed as internal state the primary and secondary. >>> In the explanation they describe that for keeping disc semantics of the >>> primary and to allow the primary to run speculatively all disc state >>> changes are directly written to the disk. In parrallel and asynchronously >>> send to the secondary. The secondary keeps the pending writing requests in >>> two disk buffers. A speculation-disk-buffer and a write-out-buffer. >>> >>> After the reception of the next checkpoint the secondary copies the >>> speculation buffer to the write out buffer, commits the checkpoint and >>> applies the write out buffer to its local disk. >>> >>> When the primary fails the secondary must wait until write-out-buffer has >>> been completely written to disk before before changing the execution mode >>> to run as primary. In this case (failure of primary) the secondary >>> discards pending disk writes in its speculation buffer. This protocol >>> keeps the disc state consistent with the last checkpoint. >>> >>> Remus uses the XEN specific blktap driver. As far as I know this can?t be >>> used with QEMU (KVM). >>> >>> I must see how drive-mirror can be used for this kind of protocol. >> >> That's all correct. Theoretically, we would do exactly the same thing: >> drive-mirror on the source would write immediately to disk but follow the >> same commit semantics on the destination as Xen. >> >>> >>> I have taken a look at COLO. >> >>> IMHO there are two points. Custom changes of the TCP-Stack are a no-go for >>> proprietary operating systems like Windows. It makes COLO application >>> agnostic but not operating system agnostic. The other point is that with >>> I/O intensive workloads COLO will tend to behave like MC. This is my point >>> of view but i didn?t invest much time to understand everything in detail. >> >> Actually, if I remember correctly, the TCP stack is only modified at the >> hypervisor level - they are intercepting and translating TCP sequence >> numbers "in-flight" to detect divergence of the source and destination - >> which is not a big problem if the implementation is well-done. > > The 2013 paper says: > 'COLO modifies the guest OS’s TCP/IP stack in order to make the behavior >more deterministic. ' > but does say that an alternative might be to have a > ' comparison function that operates transparently over re-assembled TCP > streams' > >> My hope in the future was that the two approaches could be used in a >> "Hybrid" manner - actually MC has much more of a performance hit for I/O >> than COLO does because of its buffering requirements. >> >> On the other hand, MC would perform better in a memory-intensive or >> CPU-intensive situation - so maybe QEMU could "switch" between the two >> mechanisms at different points in time when the resource bottleneck changes. > > If the primary were to rate-limit the number of resynchronisations > (and send the secondary a message as soon as it knew a resync was needed) that > would get some of the way, but then the only difference from > microcheckpointing > at that point is the secondary doing a wasteful copy and sending the packets > across; > it seems it should be easy to disable those if it knew that a resync was > going to > happen. > > Dave > >> - Michael > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 4/4] block: Drop some superfluous casts from void *
Markus Armbruster writes: > Jeff Cody writes: > >> On Mon, Aug 18, 2014 at 06:10:43PM +0200, Markus Armbruster wrote: >>> They clutter the code. Unfortunately, I can't figure out how to make >>> Coccinelle drop all of them, so I have to settle for common special >>> cases: >>> >>> @@ >>> type T; >>> T *pt; >>> void *pv; >>> @@ >>> - pt = (T *)pv; >>> + pt = pv; >>> @@ >>> type T; >>> @@ >>> - (T *) >>> (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| >>> g_try_malloc\|g_try_malloc0\|g_try_realloc\| >>> g_try_new\|g_try_new0\|g_try_renew\)(...)) >>> >>> Signed-off-by: Markus Armbruster >>> --- >>> block/vhdx-log.c| 2 +- >>> block/vvfat.c | 8 >>> hw/ide/microdrive.c | 2 +- >>> 3 files changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/block/vhdx-log.c b/block/vhdx-log.c >>> index eb5c7a0..4267e60 100644 >>> --- a/block/vhdx-log.c >>> +++ b/block/vhdx-log.c >>> @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, >>> BDRVVHDXState *s, >>> buffer = qemu_blockalign(bs, total_length); >>> memcpy(buffer, &new_hdr, sizeof(new_hdr)); >>> >>> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); >>> +new_desc = (buffer + sizeof(new_hdr)); >> >> Agree with Max, in that the parenthesis could be removed. Not worthy >> of a respin normally, but since the point of this patch is to >> unclutter the code, I guess it makes sense to fix it. > > Max and you are right. I neglected to clean up the patch produced by > spatch. Cleaned up in v3. >>> data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE); >>> data_tmp = data; >>> >>> diff --git a/block/vvfat.c b/block/vvfat.c >>> index f877e85..401539d 100644 >>> --- a/block/vvfat.c >>> +++ b/block/vvfat.c >>> @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int >>> mapping_index) >>> if(first_cluster == 0 && (is_dotdot || is_dot)) >>> continue; >>> >>> - buffer=(char*)g_malloc(length); >>> + buffer=g_malloc(length); >> >> You missed a spot to put spaces around the '=' here. Nothing worthy >> of a respin, of course - just a note in case you decide to respin. > > Right again. Although perhaps vvfat should remain ugly, to warn unwary > travellers ;) Cleaned up in v3. [...]
[Qemu-devel] [PULL v2 21/23] linux-user: Support target-to-host translation of mlockall argument
From: Tom Musta The argument to the mlockall system call is not necessarily the same on all platforms and thus may require translation prior to passing to the host. For example, PowerPC 64 bit platforms define values for MCL_CURRENT (0x2000) and MCL_FUTURE (0x4000) which are different from Intel platforms (0x1 and 0x2, respectively) Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/aarch64/syscall.h| 2 ++ linux-user/alpha/syscall.h | 2 ++ linux-user/arm/syscall.h| 2 ++ linux-user/cris/syscall.h | 2 ++ linux-user/i386/syscall.h | 2 ++ linux-user/m68k/syscall.h | 2 ++ linux-user/microblaze/syscall.h | 2 ++ linux-user/mips/syscall.h | 2 ++ linux-user/mips64/syscall.h | 2 ++ linux-user/openrisc/syscall.h | 2 ++ linux-user/ppc/syscall.h| 2 ++ linux-user/s390x/syscall.h | 2 ++ linux-user/sh4/syscall.h| 2 ++ linux-user/sparc/syscall.h | 2 ++ linux-user/sparc64/syscall.h| 2 ++ linux-user/syscall.c| 17 - linux-user/unicore32/syscall.h | 2 ++ linux-user/x86_64/syscall.h | 2 ++ 18 files changed, 50 insertions(+), 1 deletion(-) diff --git a/linux-user/aarch64/syscall.h b/linux-user/aarch64/syscall.h index d1f4823..dc72a15 100644 --- a/linux-user/aarch64/syscall.h +++ b/linux-user/aarch64/syscall.h @@ -9,3 +9,5 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "3.8.0" #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/alpha/syscall.h b/linux-user/alpha/syscall.h index 3adedeb..245cff2 100644 --- a/linux-user/alpha/syscall.h +++ b/linux-user/alpha/syscall.h @@ -253,3 +253,5 @@ struct target_pt_regs { #define TARGET_UAC_NOFIX 2 #define TARGET_UAC_SIGBUS 4 #define TARGET_MINSIGSTKSZ 4096 +#define TARGET_MLOCKALL_MCL_CURRENT 0x2000 +#define TARGET_MLOCKALL_MCL_FUTURE 0x4000 diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h index cdadb0c..3844a96 100644 --- a/linux-user/arm/syscall.h +++ b/linux-user/arm/syscall.h @@ -46,3 +46,5 @@ struct target_pt_regs { #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/cris/syscall.h b/linux-user/cris/syscall.h index a75bcc4..2957b0d 100644 --- a/linux-user/cris/syscall.h +++ b/linux-user/cris/syscall.h @@ -40,5 +40,7 @@ struct target_pt_regs { #define TARGET_CLONE_BACKWARDS2 #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 #endif diff --git a/linux-user/i386/syscall.h b/linux-user/i386/syscall.h index acf6856..906aaac 100644 --- a/linux-user/i386/syscall.h +++ b/linux-user/i386/syscall.h @@ -148,3 +148,5 @@ struct target_vm86plus_struct { #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/m68k/syscall.h b/linux-user/m68k/syscall.h index f8553f8..9218493 100644 --- a/linux-user/m68k/syscall.h +++ b/linux-user/m68k/syscall.h @@ -19,5 +19,7 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 void do_m68k_simcall(CPUM68KState *, int); diff --git a/linux-user/microblaze/syscall.h b/linux-user/microblaze/syscall.h index 2a5e160..3c1ed27 100644 --- a/linux-user/microblaze/syscall.h +++ b/linux-user/microblaze/syscall.h @@ -50,5 +50,7 @@ struct target_pt_regs { #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 #endif diff --git a/linux-user/mips/syscall.h b/linux-user/mips/syscall.h index 0b4662c..35ca23b 100644 --- a/linux-user/mips/syscall.h +++ b/linux-user/mips/syscall.h @@ -229,3 +229,5 @@ struct target_pt_regs { #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h index 39b8bed..6733107 100644 --- a/linux-user/mips64/syscall.h +++ b/linux-user/mips64/syscall.h @@ -226,3 +226,5 @@ struct target_pt_regs { #define TARGET_CLONE_BACKWARDS #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/openrisc/syscall.h b/linux-user/openrisc/syscall.h index e5e6180..8ac0365 100644 --- a/linux-user/openrisc/syscall.h +++ b/linux-user/openrisc/syscall.h @@ -25,3 +25,5 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_MINSIGSTKSZ 2048 +#define TARGET_MLOCKALL_MCL_CURRENT 1 +#define TARGET_MLOCKALL_MCL_FUTURE 2 diff --git a/linux-user/pp
[Qemu-devel] [PULL v2 19/23] linux-user: Minimum Sig Handler Stack Size for PPC64 ELF V2
From: Tom Musta The ELF V2 ABI for PPC64 defines MINSIGSTKSZ as 4096 bytes whereas it was 2048 previously. Signed-off-by: Tom Musta Signed-off-by: Riku Voipio --- linux-user/aarch64/syscall.h| 1 + linux-user/alpha/syscall.h | 1 + linux-user/arm/syscall.h| 2 ++ linux-user/cris/syscall.h | 1 + linux-user/i386/syscall.h | 1 + linux-user/m68k/syscall.h | 2 ++ linux-user/microblaze/syscall.h | 1 + linux-user/mips/syscall.h | 1 + linux-user/mips64/syscall.h | 1 + linux-user/openrisc/syscall.h | 2 ++ linux-user/ppc/syscall.h| 2 ++ linux-user/s390x/syscall.h | 1 + linux-user/sh4/syscall.h| 2 ++ linux-user/signal.c | 12 +++- linux-user/sparc/syscall.h | 1 + linux-user/sparc64/syscall.h| 1 + linux-user/unicore32/syscall.h | 2 ++ linux-user/x86_64/syscall.h | 1 + 18 files changed, 34 insertions(+), 1 deletion(-) diff --git a/linux-user/aarch64/syscall.h b/linux-user/aarch64/syscall.h index 18f44a8..d1f4823 100644 --- a/linux-user/aarch64/syscall.h +++ b/linux-user/aarch64/syscall.h @@ -8,3 +8,4 @@ struct target_pt_regs { #define UNAME_MACHINE "aarch64" #define UNAME_MINIMUM_RELEASE "3.8.0" #define TARGET_CLONE_BACKWARDS +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/alpha/syscall.h b/linux-user/alpha/syscall.h index ed13d9a..3adedeb 100644 --- a/linux-user/alpha/syscall.h +++ b/linux-user/alpha/syscall.h @@ -252,3 +252,4 @@ struct target_pt_regs { #define TARGET_UAC_NOPRINT 1 #define TARGET_UAC_NOFIX 2 #define TARGET_UAC_SIGBUS 4 +#define TARGET_MINSIGSTKSZ 4096 diff --git a/linux-user/arm/syscall.h b/linux-user/arm/syscall.h index e0d2cc3..cdadb0c 100644 --- a/linux-user/arm/syscall.h +++ b/linux-user/arm/syscall.h @@ -44,3 +44,5 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS + +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/cris/syscall.h b/linux-user/cris/syscall.h index f5783c0..a75bcc4 100644 --- a/linux-user/cris/syscall.h +++ b/linux-user/cris/syscall.h @@ -39,5 +39,6 @@ struct target_pt_regs { }; #define TARGET_CLONE_BACKWARDS2 +#define TARGET_MINSIGSTKSZ 2048 #endif diff --git a/linux-user/i386/syscall.h b/linux-user/i386/syscall.h index 9bfc1ad..acf6856 100644 --- a/linux-user/i386/syscall.h +++ b/linux-user/i386/syscall.h @@ -147,3 +147,4 @@ struct target_vm86plus_struct { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/m68k/syscall.h b/linux-user/m68k/syscall.h index 889eaf7..f8553f8 100644 --- a/linux-user/m68k/syscall.h +++ b/linux-user/m68k/syscall.h @@ -18,4 +18,6 @@ struct target_pt_regs { #define UNAME_MACHINE "m68k" #define UNAME_MINIMUM_RELEASE "2.6.32" +#define TARGET_MINSIGSTKSZ 2048 + void do_m68k_simcall(CPUM68KState *, int); diff --git a/linux-user/microblaze/syscall.h b/linux-user/microblaze/syscall.h index 5b5f6b4..2a5e160 100644 --- a/linux-user/microblaze/syscall.h +++ b/linux-user/microblaze/syscall.h @@ -49,5 +49,6 @@ struct target_pt_regs { }; #define TARGET_CLONE_BACKWARDS +#define TARGET_MINSIGSTKSZ 2048 #endif diff --git a/linux-user/mips/syscall.h b/linux-user/mips/syscall.h index 5bc5696..0b4662c 100644 --- a/linux-user/mips/syscall.h +++ b/linux-user/mips/syscall.h @@ -228,3 +228,4 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h index a7f5a58..39b8bed 100644 --- a/linux-user/mips64/syscall.h +++ b/linux-user/mips64/syscall.h @@ -225,3 +225,4 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/openrisc/syscall.h b/linux-user/openrisc/syscall.h index c3b36da..e5e6180 100644 --- a/linux-user/openrisc/syscall.h +++ b/linux-user/openrisc/syscall.h @@ -23,3 +23,5 @@ struct target_pt_regs { #define UNAME_MACHINE "openrisc" #define UNAME_MINIMUM_RELEASE "2.6.32" + +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/ppc/syscall.h b/linux-user/ppc/syscall.h index db92bbe..5311cc6 100644 --- a/linux-user/ppc/syscall.h +++ b/linux-user/ppc/syscall.h @@ -69,3 +69,5 @@ struct target_revectored_struct { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS + +#define TARGET_MINSIGSTKSZ 2048 diff --git a/linux-user/s390x/syscall.h b/linux-user/s390x/syscall.h index aaad512..b11a3b2 100644 --- a/linux-user/s390x/syscall.h +++ b/linux-user/s390x/syscall.h @@ -24,3 +24,4 @@ struct target_pt_regs { #define UNAME_MINIMUM_RELEASE "2.6.32" #define TARGET_CLONE_BACKWARDS2 +#define TARGET_MINSIGSTKSZ2048 diff --git a/linux-user/sh4/syscall.h b/linux-user/sh4/syscall.h index ccd2216..285ecf3 100644 --- a/linux-user/sh4/sy
Re: [Qemu-devel] [PATCH] net: Forbid dealing with packets when VM is not running
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: > On 2014/8/18 20:27, Dr. David Alan Gilbert wrote: > >* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: > >>For all NICs(except virtio-net) emulated by qemu, > >>Such as e1000, rtl8139, pcnet and ne2k_pci, > >>Qemu can still receive packets when VM is not running. > >>If this happened in *migration's* last PAUSE VM stage, > >>The new dirty RAM related to the packets will be missed, > >>And this will lead serious network fault in VM. > > > >I'd like to understand more about when exactly this happens. > >migration.c:migration_thread in the last step, takes the iothread, puts > >the runstate into RUN_STATE_FINISH_MIGRATE and only then calls > >qemu_savevm_state_complete > >before unlocking the iothread. > > > > Hmm, Sorry, the description above may not be exact. > > Actually, the action of receiving packets action happens after the > migration thread unlock the iothread-lock(when VM is stopped), but > before the end of the migration(to be exact, before close the socket > fd, which is used to send data to destination). > > I think before close the socket fd, we can not assure all the RAM of the > VM has been copied to the memory of network card or has been send to > the destination, we still have the chance to modify its content. (If i > am wrong, please let me know, Thanks. ;) ) > > If the above situation happens, it will dirty parts of the RAM which > will be send and parts of new dirty RAM page may be missed. As a result, > The contents of memory in the destination is not integral, And this > will lead network fault in VM. Interesting; the only reason I can think that would happen, is because arch_init.c:ram_save_page uses qemu_put_buffer_async to send most of the RAM pages and that won't make sure the write happens until later. This fix will probably help other migration code as well; postcopy runs the migration for a lot longer after stopping the CPUs, and I am seeing something that might be due to this very occasionally. Dave > Here i have made a test: > (1) Download the new qemu. > (2) Extend the time window between qemu_savevm_state_complete and > migrate_fd_cleanup(where qemu_fclose(s->file) will be called), patch > like(this will extend the chances of receiving packets between the time > window): > diff --git a/migration.c b/migration.c > index 8d675b3..597abf9 100644 > --- a/migration.c > +++ b/migration.c > @@ -614,7 +614,7 @@ static void *migration_thread(void *opaque) > qemu_savevm_state_complete(s->file); > } > qemu_mutex_unlock_iothread(); > - > +sleep(2);/*extend the time window between stop vm and end > migration*/ > if (ret < 0) { > migrate_set_state(s, MIG_STATE_ACTIVE, > MIG_STATE_ERROR); > break; > (3) Start VM (suse11sp1) and in VM ping xxx -i 0.1 > (4) Migrate the VM to another host. > > And the *PING* command in VM will very likely fail with message: > 'Destination HOST Unreachable', the NIC in VM will stay unavailable > unless you run 'service network restart'.(without step 2, you may have > to migration lots of times between two hosts before that happens). > > Thanks, > zhanghailiang > > >If that's true, how does this problem happen - can the net code > >still receive packets with the iothread lock taken? > >qemu_savevm_state_complete does a call to the RAM code, so I think this > >should get > >any RAM changes that happened before the lock was taken. > > > >I'm gently worried about threading stuff as well - is all this net code > >running off fd handlers on the main thread or are there separate threads? > > > >Dave > > > >>To avoid this, we forbid receiving packets in generic net code when > >>VM is not running. Also, when the runstate changes back to running, > >>we definitely need to flush queues to get packets flowing again. > >> > >>Here we implement this in the net layer: > >>(1) Judge the vm runstate in qemu_can_send_packet > >>(2) Add a member 'VMChangeStateEntry *vmstate' to struct NICState, > >>Which will listen for VM runstate changes. > >>(3) Register a handler function for VMstate change. > >>When vm changes back to running, we flush all queues in the callback > >>function. > >>(4) Remove checking vm state in virtio_net_can_receive > >> > >>Signed-off-by: zhanghailiang > >>--- > >> hw/net/virtio-net.c | 4 > >> include/net/net.h | 2 ++ > >> net/net.c | 32 > >> 3 files changed, 34 insertions(+), 4 deletions(-) > >> > >>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >>index 268eff9..287d762 100644 > >>--- a/hw/net/virtio-net.c > >>+++ b/hw/net/virtio-net.c > >>@@ -839,10 +839,6 @@ static int virtio_net_can_receive(NetClientState *nc) > >> VirtIODevice *vdev = VIRTIO_DEVICE(n); > >> VirtIONetQueue *q = virtio_net_get_subqueue(nc); > >> > >>-if (!vdev->vm_running) { > >>-return 0; > >
Re: [Qemu-devel] Microcheckpointing: Memory-VCPU / Disk State consistency
Hi Paolo, thanks for your hint. I missed your mail from last sunday. I will take a look on that! Walid > Am 17.08.2014 um 11:52 schrieb Paolo Bonzini : > > Il 11/08/2014 22:15, Michael R. Hines ha scritto: >> Excellent question: QEMU does have a feature called "drive-mirror" >> in block/mirror.c that was introduced a couple of years ago. I'm not >> sure what the >> adoption rate of the feature is, but I would start with that one. > > block/mirror.c is asynchronous, and there's no support for communicating > checkpoints back to the master. However, the quorum disk driver could > be what you need. > > There's also a series on the mailing list that lets quorum read only > from the primary, so that quorum can still do replication and fault > tolerance, but skip fault detection. > > Paolo > >> There is also a second fault tolerance implementation that works a >> little differently called >> "COLO" - you may have seen those emails on the list too, but their >> method does not require a disk replication solution, if I recall correctly. >
Re: [Qemu-devel] [PATCH V2 0/2] runner: Control test duration
On Tue, Aug 19, 2014 at 12:02:33AM +0400, Maria Kustova wrote: > The first patch adds the '--duration SECONDS' argument. After the specified > duration the runner allows to end the current test and then exits. > > The second patch adds forced termination of a program under test, if the test > execution takes more than 10 minutes to indicate program freezes. > > If a program under test hangs, then the specified test duration can be overrun > up to 10 minutes. > > The patch series is based on https://github.com/stefanha/qemu/commits/block, > commit 07a45925fa88376f8583a333e74f7eeb0f455685 > > v1 -> v2: > * Trivial fixes based on the review of Fam Zheng > * Increased time-out (in some cases 5 minutes interval returned false >negatives) > > Maria Kustova (2): > runner: Add an argument for test duration > runner: Kill a program under test by time-out > > tests/image-fuzzer/runner.py | 50 > +--- > 1 file changed, 42 insertions(+), 8 deletions(-) Reviewed-by: Stefan Hajnoczi pgpQnasBDCwmO.pgp Description: PGP signature
Re: [Qemu-devel] [PULL] virtio-serial: avoid duplicate port names
On 18 August 2014 18:18, Amit Shah wrote: > Hi, > > Patches have been on list for a while.. > > The following changes since commit 08ab59770da57648bfb8fc9be37f0ef7fb50b0f9: > > Merge remote-tracking branch 'remotes/mcayland/qemu-sparc' into staging > (2014-08-18 12:55:02 +0100) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git for-2.2 > > for you to fetch changes up to d0a0bfe6729ef6044d76ea49fafa07e29fa598bd: > > virtio-serial: search for duplicate port names before adding new ports > (2014-08-18 22:42:49 +0530) > > > Amit Shah (2): > virtio-serial: create a linked list of all active devices > virtio-serial: search for duplicate port names before adding new ports > > hw/char/virtio-serial-bus.c | 32 > include/hw/virtio/virtio-serial.h | 2 ++ > 2 files changed, 34 insertions(+) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 3/3] layout: Add generators for refcount tbles and blocks
On Mon, 08/11 15:55, Maria Kustova wrote: > Refcount structures are placed in clusters randomly selected from all not > allocated host clusters. s/not allocated/unallocated/ > > Signed-off-by: Maria Kustova > --- > tests/image-fuzzer/qcow2/layout.py | 136 > - > 1 file changed, 135 insertions(+), 1 deletion(-) > > diff --git a/tests/image-fuzzer/qcow2/layout.py > b/tests/image-fuzzer/qcow2/layout.py > index 730c771..2239789 100644 > --- a/tests/image-fuzzer/qcow2/layout.py > +++ b/tests/image-fuzzer/qcow2/layout.py > @@ -102,6 +102,8 @@ class Image(object): > self.end_of_extension_area = FieldsList() > self.l2_tables = FieldsList() > self.l1_table = FieldsList() > +self.refcount_table = FieldsList() > +self.refcount_blocks = FieldsList() > self.ext_offset = 0 > self.create_header(cluster_bits, backing_file_name) > self.set_backing_file_name(backing_file_name) > @@ -113,7 +115,8 @@ class Image(object): > def __iter__(self): > return chain(self.header, self.backing_file_format, > self.feature_name_table, self.end_of_extension_area, > - self.backing_file_name, self.l1_table, self.l2_tables) > + self.backing_file_name, self.l1_table, self.l2_tables, > + self.refcount_table, self.refcount_blocks) > > def create_header(self, cluster_bits, backing_file_name=None): > """Generate a random valid header.""" > @@ -330,6 +333,136 @@ class Image(object): > float(self.cluster_size**2))) > self.header['l1_table_offset'][0].value = l1_offset > > +def create_refcount_structures(self): > +"""Generate random refcount blocks and refcount table.""" > +def allocate_rfc_blocks(data, size): > +"""Return indices of clusters allocated for recount blocks.""" s/recount/refcount/ > +cluster_ids = set() > +diff = block_ids = set([x / size for x in data]) > +while len(diff) != 0: > +# Allocate all yet not allocated clusters > +new = self._get_available_clusters(data | cluster_ids, > + len(diff)) > +# Indices of new refcount blocks necessary to cover clusters > +# in 'new' > +diff = set([x / size for x in new]) - block_ids > +cluster_ids |= new > +block_ids |= diff > +return cluster_ids, block_ids > + > +def allocate_rfc_table(data, init_blocks, block_size): > +"""Return indices of clusters allocated for the refcount table > +and updated indices of clusters allocated for blocks and indices > +of blocks. > +""" > +blocks = set(init_blocks) > +clusters = set() > +# Number of entries in one cluster of the refcount table > +size = self.cluster_size / UINT64_S > +# Number of clusters necessary for the refcount table based on > +# the current number of refcount blocks > +table_size = int(ceil((max(blocks) + 1) / float(size))) > +# Index of the first cluster of the refcount table > +table_start = self._get_adjacent_clusters(data, table_size + 1) > +# Clusters allocated for the current length of the refcount table > +table_clusters = set(range(table_start, table_start + > table_size)) > +# Clusters allocated for the refcount table including > +# last optional one for potential l1 growth > +table_clusters_allocated = set(range(table_start, table_start + > + table_size + 1)) > +# New refcount blocks necessary for clusters occupied by the > +# refcount table > +diff = set([c / block_size for c in table_clusters]) - blocks > +blocks |= diff > +while len(diff) != 0: > +# Allocate clusters for new refcount blocks > +new = self._get_available_clusters((data | clusters) | > + table_clusters_allocated, > + len(diff)) > +# Indices of new refcount blocks necessary to cover > +# clusters in 'new' > +diff = set([x / block_size for x in new]) - blocks > +clusters |= new > +blocks |= diff > +# Check if the refcount table needs one more cluster > +if int(ceil((max(blocks) + 1) / float(size))) > table_size: > +new_block_id = (table_start + table_size) / block_size > +# Check if the additional table cluster needs > +# o
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote: > diff --git a/tests/image-fuzzer/qcow2/fuzz.py > b/tests/image-fuzzer/qcow2/fuzz.py > index 6e272c6..c652dc9 100644 > --- a/tests/image-fuzzer/qcow2/fuzz.py > +++ b/tests/image-fuzzer/qcow2/fuzz.py > @@ -123,7 +123,7 @@ def string_validator(current, strings): > return validator(current, random.choice, strings) > > > -def selector(current, constraints, validate=int_validator): > +def selector(current, constraints, validate=None): > """Select one value from all defined by constraints. > > Each constraint produces one random value satisfying to it. The function > @@ -131,6 +131,9 @@ def selector(current, constraints, > validate=int_validator): > constraints overlaps). > """ > > +if validate is None: > +validate = int_validator > + > def iter_validate(c): > """Apply validate() only to constraints represented as lists. > I don't understand the benefit of this change. The default initializer syntax did what was intended, now it has been made more complicated with a None value that gets converted to int_validator later. pgpNVPpauhild.pgp Description: PGP signature
[Qemu-devel] [PATCH 3/3] pcie: using error_setg instead of impolite assert
From: Gonglei It's enough of reporting an error. Assert() is not acceptable because the error is not a fatal error. Signed-off-by: Gonglei --- hw/pci/pcie.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index a123c01..7b46140 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -254,7 +254,11 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, * Right now, only a device of function = 0 is allowed to be * hot plugged/unplugged. */ -assert(PCI_FUNC(pci_dev->devfn) == 0); +if (PCI_FUNC(pci_dev->devfn) != 0) { +error_setg(errp, "Unsupported device function %d for PCIe hotplugging, " + "only supported function 0", PCI_FUNC(pci_dev->devfn)); +return; +} pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDS); -- 1.7.12.4
[Qemu-devel] [PATCH 2/3] qdev: Refactor device_set_realized to avoid resource leak
From: Gonglei At present, the local variable local_err is reused at multi-places, Which will cause resource leak in some scenarios. Example: 1. Assuming that "dc->realize(dev, &local_err)" execute successful and local_err == NULL; 2. Executing device hotplug in hotplug_handler_plug(), but failed (It is prone to occur). Then local_err != NULL; 3. error_propagate(errp, local_err) and return. But the resources which been allocated in dc->realize() will be leaked. Simple backtrace: dc->realize() |->device_realize |->pci_qdev_init() |->do_pci_register_device() |->etc. To avoid the resource leak, using some different local error variables. For different error conditions, release the corresponding resources. Signed-off-by: Gonglei --- hw/core/qdev.c | 75 +++--- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 3e7085e..b3a463b 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -839,41 +839,58 @@ static void device_set_realized(Object *obj, bool value, Error **errp) dc->realize(dev, &local_err); } -if (dev->parent_bus && dev->parent_bus->hotplug_handler && -local_err == NULL) { -hotplug_handler_plug(dev->parent_bus->hotplug_handler, - dev, &local_err); -} else if (local_err == NULL && - object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { -HotplugHandler *hotplug_ctrl; -MachineState *machine = MACHINE(qdev_get_machine()); -MachineClass *mc = MACHINE_GET_CLASS(machine); - -if (mc->get_hotplug_handler) { -hotplug_ctrl = mc->get_hotplug_handler(machine, dev); -if (hotplug_ctrl) { -hotplug_handler_plug(hotplug_ctrl, dev, &local_err); +if (local_err == NULL) { +Error *hotplug_err = NULL; + +if (dev->parent_bus && dev->parent_bus->hotplug_handler) { +hotplug_handler_plug(dev->parent_bus->hotplug_handler, + dev, &hotplug_err); +} else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { +HotplugHandler *hotplug_ctrl; +MachineState *machine = MACHINE(qdev_get_machine()); +MachineClass *mc = MACHINE_GET_CLASS(machine); + +if (mc->get_hotplug_handler) { +hotplug_ctrl = mc->get_hotplug_handler(machine, dev); +if (hotplug_ctrl) { +hotplug_handler_plug(hotplug_ctrl, dev, &hotplug_err); +} } } -} -if (qdev_get_vmsd(dev) && local_err == NULL) { -vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev, - dev->instance_id_alias, - dev->alias_required_for_version); -} -if (local_err == NULL) { -QLIST_FOREACH(bus, &dev->child_bus, sibling) { -object_property_set_bool(OBJECT(bus), true, "realized", - &local_err); -if (local_err != NULL) { -break; +if (hotplug_err == NULL) { +Error *err = NULL; +if (qdev_get_vmsd(dev)) { +vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), + dev, dev->instance_id_alias, + dev->alias_required_for_version); } + +QLIST_FOREACH(bus, &dev->child_bus, sibling) { +object_property_set_bool(OBJECT(bus), true, "realized", + &err); +if (err != NULL) { +if (qdev_get_vmsd(dev)) { +vmstate_unregister(dev, qdev_get_vmsd(dev), dev); +} + +break; +} +} + +if (dev->hotplugged && err == NULL) { +device_reset(dev); +} +error_free(err); +} else { +if (dc->unrealize) { +dc->unrealize(dev, NULL); +} + +error_propagate(errp, hotplug_err); +return; } } -if (dev->hotplugged && local_err == NULL) { -device_reset(dev); -} dev->pending_deleted_event = false; } else if (!value && dev->realized) { QLIST_FOREACH(bus, &dev->child_bus, sibling) { -- 1.7.12.4
[Qemu-devel] [PATCH 1/3] qdev: add missing error check
From: Gonglei If local_err is not null, the next code logic is useless. Signed-off-by: Gonglei --- hw/core/qdev.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index da1ba48..3e7085e 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -830,6 +830,11 @@ static void device_set_realized(Object *obj, bool value, Error **errp) g_free(name); } +if (local_err != NULL) { +error_propagate(errp, local_err); +return; +} + if (dc->realize) { dc->realize(dev, &local_err); } -- 1.7.12.4
[Qemu-devel] [PATCH 0/3] Refactor device_set_realized to avoid resource leak.
From: Gonglei after committing [PATCH v6 0/9] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API if devcie hotplgging failed, will casuse resource leak. This patch series include address resouce leak and two other issuses. BTW, for patch 2/3, checkpatch.py report a warning, but I have no idea how to handle this probleam. Any ideas? WARNING: line over 80 characters #90: FILE: hw/core/qdev.c:866: + dev->alias_required_for_version); total: 0 errors, 1 warnings, 87 lines checked Please review, thanks in advance. Gonglei (3): qdev: add missing error check qdev: Refactor device_set_realized to avoid resource pcie: using error_setg instead of impolite assert hw/core/qdev.c | 80 +- hw/pci/pcie.c | 6 - 2 files changed, 56 insertions(+), 30 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
On Tue, 08/19 02:00, Maria Kustova wrote: > Signed-off-by: Maria Kustova > --- > tests/image-fuzzer/qcow2/fuzz.py | 15 ++-- > tests/image-fuzzer/runner.py | 51 > > 2 files changed, 34 insertions(+), 32 deletions(-) > > diff --git a/tests/image-fuzzer/qcow2/fuzz.py > b/tests/image-fuzzer/qcow2/fuzz.py > index 6e272c6..c652dc9 100644 > --- a/tests/image-fuzzer/qcow2/fuzz.py > +++ b/tests/image-fuzzer/qcow2/fuzz.py > @@ -123,7 +123,7 @@ def string_validator(current, strings): > return validator(current, random.choice, strings) > > > -def selector(current, constraints, validate=int_validator): > +def selector(current, constraints, validate=None): > """Select one value from all defined by constraints. > > Each constraint produces one random value satisfying to it. The function > @@ -131,6 +131,9 @@ def selector(current, constraints, > validate=int_validator): > constraints overlaps). > """ > > +if validate is None: > +validate = int_validator > + > def iter_validate(c): > """Apply validate() only to constraints represented as lists. > > @@ -337,9 +340,8 @@ def l1_entry(current): > constraints = UINT64_V > # Reserved bits are ignored > # Added a possibility when only flags are fuzzed > -offset = 0x7fff & random.choice([selector(current, > - constraints), > - current]) > +offset = 0x7fff & \ > + random.choice([selector(current, constraints), current]) > is_cow = random.randint(0, 1) > return offset + (is_cow << UINT64_M) > > @@ -349,9 +351,8 @@ def l2_entry(current): > constraints = UINT64_V > # Reserved bits are ignored > # Add a possibility when only flags are fuzzed > -offset = 0x3ffe & random.choice([selector(current, > - constraints), > - current]) > +offset = 0x3ffe & \ > + random.choice([selector(current, constraints), current]) > is_compressed = random.randint(0, 1) > is_cow = random.randint(0, 1) > is_zero = random.randint(0, 1) > diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py > index fd97c40..b142577 100755 > --- a/tests/image-fuzzer/runner.py > +++ b/tests/image-fuzzer/runner.py > @@ -73,7 +73,7 @@ def run_app(fd, q_args): > """Exception for signal.alarm events.""" > pass > > -def handler(*arg): > +def handler(*args): > """Notify that an alarm event occurred.""" > raise Alarm > > @@ -137,8 +137,8 @@ class TestEnv(object): > self.init_path = os.getcwd() > self.work_dir = work_dir > self.current_dir = os.path.join(work_dir, 'test-' + test_id) > -self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\ > - .strip().split(' ') > +self.qemu_img = \ > +os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ') > self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' > ') > strings = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x', > '%d%d%d%d', '%s%s%s%s', '%999s', '%08x', '%%20d', > @@ -247,10 +247,8 @@ class TestEnv(object): > > os.chdir(self.current_dir) > backing_file_name, backing_file_fmt = self._create_backing_file() > -img_size = image_generator.create_image('test.img', > -backing_file_name, > -backing_file_fmt, > -fuzz_config) Actually this one is fine, but the new way is good too. > +img_size = image_generator.create_image( > +'test.img', backing_file_name, backing_file_fmt, fuzz_config) > for item in commands: > shutil.copy('test.img', 'copy.img') > # 'off' and 'len' are multiple of the sector size > @@ -263,7 +261,7 @@ class TestEnv(object): > elif item[0] == 'qemu-io': > current_cmd = list(self.qemu_io) > else: > -multilog("Warning: test command '%s' is not defined.\n" \ > +multilog("Warning: test command '%s' is not defined.\n" > % item[0], sys.stderr, self.log, self.parent_log) > continue > # Replace all placeholders with their real values > @@ -279,29 +277,30 @@ class TestEnv(object): > "Backing file: %s\n" \ > % (self.seed, " ".join(current_cmd), >self.current_dir, backing_file_name) > - > temp_log = StringIO.StringIO() > try: >
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
On Tue, Aug 19, 2014 at 1:38 PM, Stefan Hajnoczi wrote: > On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote: >> diff --git a/tests/image-fuzzer/qcow2/fuzz.py >> b/tests/image-fuzzer/qcow2/fuzz.py >> index 6e272c6..c652dc9 100644 >> --- a/tests/image-fuzzer/qcow2/fuzz.py >> +++ b/tests/image-fuzzer/qcow2/fuzz.py >> @@ -123,7 +123,7 @@ def string_validator(current, strings): >> return validator(current, random.choice, strings) >> >> >> -def selector(current, constraints, validate=int_validator): >> +def selector(current, constraints, validate=None): >> """Select one value from all defined by constraints. >> >> Each constraint produces one random value satisfying to it. The function >> @@ -131,6 +131,9 @@ def selector(current, constraints, >> validate=int_validator): >> constraints overlaps). >> """ >> >> +if validate is None: >> +validate = int_validator >> + >> def iter_validate(c): >> """Apply validate() only to constraints represented as lists. >> > > I don't understand the benefit of this change. The default initializer > syntax did what was intended, now it has been made more complicated with > a None value that gets converted to int_validator later. It's a convention rule that allows keep the function signature unchanged in case of modified function internals. In other words, wrapper functions should not change in response of the internal function change. Maria.
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
On Tue, Aug 19, 2014 at 1:44 PM, Fam Zheng wrote: > On Tue, 08/19 02:00, Maria Kustova wrote: >> Signed-off-by: Maria Kustova >> --- >> tests/image-fuzzer/qcow2/fuzz.py | 15 ++-- >> tests/image-fuzzer/runner.py | 51 >> >> 2 files changed, 34 insertions(+), 32 deletions(-) >> >> diff --git a/tests/image-fuzzer/qcow2/fuzz.py >> b/tests/image-fuzzer/qcow2/fuzz.py >> index 6e272c6..c652dc9 100644 >> --- a/tests/image-fuzzer/qcow2/fuzz.py >> +++ b/tests/image-fuzzer/qcow2/fuzz.py >> @@ -123,7 +123,7 @@ def string_validator(current, strings): >> return validator(current, random.choice, strings) >> >> >> -def selector(current, constraints, validate=int_validator): >> +def selector(current, constraints, validate=None): >> """Select one value from all defined by constraints. >> >> Each constraint produces one random value satisfying to it. The function >> @@ -131,6 +131,9 @@ def selector(current, constraints, >> validate=int_validator): >> constraints overlaps). >> """ >> >> +if validate is None: >> +validate = int_validator >> + >> def iter_validate(c): >> """Apply validate() only to constraints represented as lists. >> >> @@ -337,9 +340,8 @@ def l1_entry(current): >> constraints = UINT64_V >> # Reserved bits are ignored >> # Added a possibility when only flags are fuzzed >> -offset = 0x7fff & random.choice([selector(current, >> - constraints), >> - current]) >> +offset = 0x7fff & \ >> + random.choice([selector(current, constraints), current]) >> is_cow = random.randint(0, 1) >> return offset + (is_cow << UINT64_M) >> >> @@ -349,9 +351,8 @@ def l2_entry(current): >> constraints = UINT64_V >> # Reserved bits are ignored >> # Add a possibility when only flags are fuzzed >> -offset = 0x3ffe & random.choice([selector(current, >> - constraints), >> - current]) >> +offset = 0x3ffe & \ >> + random.choice([selector(current, constraints), current]) >> is_compressed = random.randint(0, 1) >> is_cow = random.randint(0, 1) >> is_zero = random.randint(0, 1) >> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py >> index fd97c40..b142577 100755 >> --- a/tests/image-fuzzer/runner.py >> +++ b/tests/image-fuzzer/runner.py >> @@ -73,7 +73,7 @@ def run_app(fd, q_args): >> """Exception for signal.alarm events.""" >> pass >> >> -def handler(*arg): >> +def handler(*args): >> """Notify that an alarm event occurred.""" >> raise Alarm >> >> @@ -137,8 +137,8 @@ class TestEnv(object): >> self.init_path = os.getcwd() >> self.work_dir = work_dir >> self.current_dir = os.path.join(work_dir, 'test-' + test_id) >> -self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\ >> - .strip().split(' ') >> +self.qemu_img = \ >> +os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ') >> self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' >> ') >> strings = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x', >> '%d%d%d%d', '%s%s%s%s', '%999s', '%08x', '%%20d', >> @@ -247,10 +247,8 @@ class TestEnv(object): >> >> os.chdir(self.current_dir) >> backing_file_name, backing_file_fmt = self._create_backing_file() >> -img_size = image_generator.create_image('test.img', >> -backing_file_name, >> -backing_file_fmt, >> -fuzz_config) > > Actually this one is fine, but the new way is good too. > >> +img_size = image_generator.create_image( >> +'test.img', backing_file_name, backing_file_fmt, fuzz_config) >> for item in commands: >> shutil.copy('test.img', 'copy.img') >> # 'off' and 'len' are multiple of the sector size >> @@ -263,7 +261,7 @@ class TestEnv(object): >> elif item[0] == 'qemu-io': >> current_cmd = list(self.qemu_io) >> else: >> -multilog("Warning: test command '%s' is not defined.\n" \ >> +multilog("Warning: test command '%s' is not defined.\n" >> % item[0], sys.stderr, self.log, self.parent_log) >> continue >> # Replace all placeholders with their real values >> @@ -279,29 +277,30 @@ class TestEnv(object): >> "Backing file: %s\n" \ >> % (self.seed, " ".join(current_cmd), >>
Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code
On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote: > Implement ARMv8 software single-step handling for A64 code: > correctly update the single-step state machine and generate > debug exceptions when stepping A64 code. > > This patch has no behavioural change since MDSCR_EL1.SS can't > be set by the guest yet. Hi Peter, > > Signed-off-by: Peter Maydell > --- > target-arm/cpu.h | 21 +++ > target-arm/helper.h| 1 + > target-arm/internals.h | 6 +++ > target-arm/op_helper.c | 5 +++ > target-arm/translate-a64.c | 91 > +++--- > target-arm/translate.h | 12 ++ > 6 files changed, 131 insertions(+), 5 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 74f7b15..ac01524 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState > *env) > #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT) > #define ARM_TBFLAG_AA64_FPEN_SHIFT 2 > #define ARM_TBFLAG_AA64_FPEN_MASK (1 << ARM_TBFLAG_AA64_FPEN_SHIFT) > +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3 > +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT) > +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3 > +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << ARM_TBFLAG_AA64_PSTATE_SS_SHIFT) Shouldn't these shifts/masks differ? > > /* some convenience accessor macros */ > #define ARM_TBFLAG_AARCH64_STATE(F) \ > @@ -1235,6 +1239,10 @@ static inline bool arm_singlestep_active(CPUARMState > *env) > (((F) & ARM_TBFLAG_AA64_EL_MASK) >> ARM_TBFLAG_AA64_EL_SHIFT) > #define ARM_TBFLAG_AA64_FPEN(F) \ > (((F) & ARM_TBFLAG_AA64_FPEN_MASK) >> ARM_TBFLAG_AA64_FPEN_SHIFT) > +#define ARM_TBFLAG_AA64_SS_ACTIVE(F) \ > +(((F) & ARM_TBFLAG_AA64_SS_ACTIVE_MASK) >> > ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT) > +#define ARM_TBFLAG_AA64_PSTATE_SS(F) \ > +(((F) & ARM_TBFLAG_AA64_PSTATE_SS_MASK) >> > ARM_TBFLAG_AA64_PSTATE_SS_SHIFT) > > static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, > target_ulong *cs_base, int *flags) > @@ -1248,6 +1256,19 @@ static inline void cpu_get_tb_cpu_state(CPUARMState > *env, target_ulong *pc, > if (fpen == 3 || (fpen == 1 && arm_current_pl(env) != 0)) { > *flags |= ARM_TBFLAG_AA64_FPEN_MASK; > } > +/* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine > + * states defined in the ARM ARM for software singlestep: > + * SS_ACTIVE PSTATE.SS State > + * 0x Inactive (the TB flag for SS is always 0) > + * 10 Active-pending > + * 11 Active-not-pending > + */ > +if (arm_singlestep_active(env)) { > +*flags |= ARM_TBFLAG_AA64_SS_ACTIVE_MASK; > +if (env->pstate & PSTATE_SS) { > +*flags |= ARM_TBFLAG_AA64_PSTATE_SS_MASK; > +} > +} > } else { > int privmode; > *pc = env->regs[15]; > diff --git a/target-arm/helper.h b/target-arm/helper.h > index facfcd2..1d7003b 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -64,6 +64,7 @@ DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) > DEF_HELPER_2(get_cp_reg64, i64, env, ptr) > > DEF_HELPER_3(msr_i_pstate, void, env, i32, i32) > +DEF_HELPER_1(clear_pstate_ss, void, env) > DEF_HELPER_1(exception_return, void, env) > > DEF_HELPER_2(get_r13_banked, i32, env, i32) > diff --git a/target-arm/internals.h b/target-arm/internals.h > index 08fa697..53c2e3c 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -290,4 +290,10 @@ static inline uint32_t syn_data_abort(int same_el, int > ea, int cm, int s1ptw, > | (ea << 9) | (cm << 8) | (s1ptw << 7) | (wnr << 6) | fsc; > } > > +static inline uint32_t syn_swstep(int same_el, int isv, int ex) > +{ > +return (EC_SOFTWARESTEP << ARM_EL_EC_SHIFT) | (same_el << > ARM_EL_EC_SHIFT) > +| (isv << 24) | (ex << 6) | 0x22; > +} > + > #endif > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 62cc07d..fe40358 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -369,6 +369,11 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, > uint32_t imm) > } > } > > +void HELPER(clear_pstate_ss)(CPUARMState *env) > +{ > +env->pstate &= ~PSTATE_SS; > +} > + > void HELPER(exception_return)(CPUARMState *env) > { > int cur_el = arm_current_pl(env); > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index aa731bf..21cb12b 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -203,10 +203,39 @@ static void gen_exception_insn(DisasContext *s, int > offset, int excp, > s->is_jmp = DISAS_EXC; > } > > +static void gen_ss_advance(DisasContext *s) > +{ > +/* I
Re: [Qemu-devel] [PATCH 3/3] layout: Add generators for refcount tbles and blocks
On Tue, Aug 19, 2014 at 1:36 PM, Fam Zheng wrote: > On Mon, 08/11 15:55, Maria Kustova wrote: >> Refcount structures are placed in clusters randomly selected from all not >> allocated host clusters. > > s/not allocated/unallocated/ > >> >> Signed-off-by: Maria Kustova >> --- >> tests/image-fuzzer/qcow2/layout.py | 136 >> - >> 1 file changed, 135 insertions(+), 1 deletion(-) >> >> diff --git a/tests/image-fuzzer/qcow2/layout.py >> b/tests/image-fuzzer/qcow2/layout.py >> index 730c771..2239789 100644 >> --- a/tests/image-fuzzer/qcow2/layout.py >> +++ b/tests/image-fuzzer/qcow2/layout.py >> @@ -102,6 +102,8 @@ class Image(object): >> self.end_of_extension_area = FieldsList() >> self.l2_tables = FieldsList() >> self.l1_table = FieldsList() >> +self.refcount_table = FieldsList() >> +self.refcount_blocks = FieldsList() >> self.ext_offset = 0 >> self.create_header(cluster_bits, backing_file_name) >> self.set_backing_file_name(backing_file_name) >> @@ -113,7 +115,8 @@ class Image(object): >> def __iter__(self): >> return chain(self.header, self.backing_file_format, >> self.feature_name_table, self.end_of_extension_area, >> - self.backing_file_name, self.l1_table, self.l2_tables) >> + self.backing_file_name, self.l1_table, self.l2_tables, >> + self.refcount_table, self.refcount_blocks) >> >> def create_header(self, cluster_bits, backing_file_name=None): >> """Generate a random valid header.""" >> @@ -330,6 +333,136 @@ class Image(object): >> >> float(self.cluster_size**2))) >> self.header['l1_table_offset'][0].value = l1_offset >> >> +def create_refcount_structures(self): >> +"""Generate random refcount blocks and refcount table.""" >> +def allocate_rfc_blocks(data, size): >> +"""Return indices of clusters allocated for recount blocks.""" > > s/recount/refcount/ > >> +cluster_ids = set() >> +diff = block_ids = set([x / size for x in data]) >> +while len(diff) != 0: >> +# Allocate all yet not allocated clusters >> +new = self._get_available_clusters(data | cluster_ids, >> + len(diff)) >> +# Indices of new refcount blocks necessary to cover clusters >> +# in 'new' >> +diff = set([x / size for x in new]) - block_ids >> +cluster_ids |= new >> +block_ids |= diff >> +return cluster_ids, block_ids >> + >> +def allocate_rfc_table(data, init_blocks, block_size): >> +"""Return indices of clusters allocated for the refcount table >> +and updated indices of clusters allocated for blocks and indices >> +of blocks. >> +""" >> +blocks = set(init_blocks) >> +clusters = set() >> +# Number of entries in one cluster of the refcount table >> +size = self.cluster_size / UINT64_S >> +# Number of clusters necessary for the refcount table based on >> +# the current number of refcount blocks >> +table_size = int(ceil((max(blocks) + 1) / float(size))) >> +# Index of the first cluster of the refcount table >> +table_start = self._get_adjacent_clusters(data, table_size + 1) >> +# Clusters allocated for the current length of the refcount >> table >> +table_clusters = set(range(table_start, table_start + >> table_size)) >> +# Clusters allocated for the refcount table including >> +# last optional one for potential l1 growth >> +table_clusters_allocated = set(range(table_start, table_start + >> + table_size + 1)) >> +# New refcount blocks necessary for clusters occupied by the >> +# refcount table >> +diff = set([c / block_size for c in table_clusters]) - blocks >> +blocks |= diff >> +while len(diff) != 0: >> +# Allocate clusters for new refcount blocks >> +new = self._get_available_clusters((data | clusters) | >> + table_clusters_allocated, >> + len(diff)) >> +# Indices of new refcount blocks necessary to cover >> +# clusters in 'new' >> +diff = set([x / block_size for x in new]) - blocks >> +clusters |= new >> +blocks |= diff >> +# Check if the refcount table needs one more cluster >> +if int(ceil((max(blocks) + 1) / float(size))) > table_size: >> +
Re: [Qemu-devel] KVM call for agenda for 2014-08-19
Juan Quintela wrote: > Hi > > Please, send any topic that you are interested in covering. > > People have complained on the past that I don't cancel the call until > the very last minute. So, what do you think that deadline for > submitting topics is 23:00UTC on Monday? As there are no topics, call gets cancelled. Have a good week, Juan.
Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code
On 19 August 2014 10:56, Edgar E. Iglesias wrote: > On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote: >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState >> *env) >> #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT) >> #define ARM_TBFLAG_AA64_FPEN_SHIFT 2 >> #define ARM_TBFLAG_AA64_FPEN_MASK (1 << ARM_TBFLAG_AA64_FPEN_SHIFT) >> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3 >> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << >> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT) >> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3 >> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << >> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT) > > Shouldn't these shifts/masks differ? Oops. Yes, they certainly should. -- PMM
Re: [Qemu-devel] [PATCH 0/3] image-fuzzer: Support refcount structures in the qcow2 image generator
On Mon, Aug 11, 2014 at 03:55:03PM +0400, Maria Kustova wrote: > This patch series adds support of refcount tables and blocks to the qcow2 > image > generator. > > This patch series was created for the 'block-next' branch and based on the > next > series: > [PATCH V3] layout: Reduce number of generator functions in __init__ > > Maria Kustova (3): > docs: List all image elements currently supported by the fuzzer > fuzz: Add fuzzing functions for entries of refcount table and blocks > layout: Add generators for refcount tbles and blocks > > docs/image-fuzzer.txt | 3 +- > tests/image-fuzzer/qcow2/fuzz.py | 16 - > tests/image-fuzzer/qcow2/layout.py | 136 > - > 3 files changed, 152 insertions(+), 3 deletions(-) > > -- > 1.9.3 > Reviewed-by: Stefan Hajnoczi pgpc49clynS3v.pgp Description: PGP signature
[Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18
The following changes since commit 5a7348045091a2bc15d85bb177e5956aa6114e5a: Update version for v2.1.0-rc2 release (2014-07-15 18:55:37 +0100) are available in the git repository at: git://github.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to f54bb15f9d373877954e44db3a8bb368aff45b42: mtree: remove write-only field (2014-08-18 12:06:21 +0200) SCSI changes that enable sending vendor-specific commands via virtio-scsi. Memory changes for QOMification and automatic tracking of MR lifetime. Paolo Bonzini (15): scsi-bus: prepare scsi_req_new for introduction of parse_cdb scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo scsi-block: extract scsi_block_is_passthrough scsi-block, scsi-generic: implement parse_cdb virtio-scsi: implement parse_cdb qom: object: delete properties before calling instance_finalize qom: object: move unparenting to the child property's release callback sysbus: remove unused function sysbus_del_io vga: do not dynamically allocate chain4_alias nic: do not destroy memory regions in cleanup functions ioport: split deletion and destruction memory: convert memory_region_destroy to object_unparent memory: remove memory_region_destroy tpm_tis: remove instance_finalize callback mtree: remove write-only field Peter Crosthwaite (5): loader: Abstract away ref to memory region names exec: Abstract away ref to memory region names memory: constify memory_region_name memory: Use memory_region_name for name access memory: Use canonical path component as the name backends/hostmem.c | 10 -- docs/memory.txt| 15 ++--- exec.c | 4 +-- hw/audio/ac97.c| 9 - hw/audio/es1370.c | 8 - hw/audio/intel-hda.c | 1 - hw/block/nvme.c| 1 - hw/block/pflash_cfi01.c| 1 - hw/block/pflash_cfi02.c| 1 - hw/char/serial-pci.c | 3 -- hw/core/loader.c | 2 +- hw/core/sysbus.c | 5 --- hw/display/vga.c | 24 ++ hw/display/vga_int.h | 3 +- hw/i386/kvm/pci-assign.c | 8 - hw/i386/kvmvapic.c | 2 +- hw/ide/ahci.c | 2 -- hw/ide/cmd646.c| 5 --- hw/ide/piix.c | 3 -- hw/ide/via.c | 3 -- hw/ipack/tpci200.c | 13 hw/mips/gt64xxx_pci.c | 2 +- hw/misc/ivshmem.c | 3 -- hw/misc/omap_gpmc.c| 2 +- hw/misc/pci-testdev.c | 2 -- hw/misc/vfio.c | 11 ++- hw/net/dp8393x.c | 3 -- hw/net/e1000.c | 2 -- hw/net/eepro100.c | 3 -- hw/net/mcf_fec.c | 3 -- hw/net/ne2000.c| 1 - hw/net/pcnet-pci.c | 2 -- hw/net/rtl8139.c | 2 -- hw/net/stellaris_enet.c| 8 - hw/net/vmxnet3.c | 4 --- hw/pci-bridge/pci_bridge_dev.c | 2 -- hw/pci/msix.c | 4 --- hw/pci/pci.c | 2 -- hw/pci/pci_bridge.c| 8 - hw/pci/pcie_host.c | 1 - hw/pci/shpc.c | 1 - hw/ppc/ppc4xx_devs.c | 2 +- hw/scsi/esp-pci.c | 1 - hw/scsi/lsi53c895a.c | 10 -- hw/scsi/megasas.c | 3 -- hw/scsi/scsi-bus.c | 74 +- hw/scsi/scsi-disk.c| 52 ++--- hw/scsi/scsi-generic.c | 7 hw/scsi/virtio-scsi.c | 25 ++ hw/scsi/vmw_pvscsi.c | 3 -- hw/tpm/tpm_tis.c | 9 - hw/usb/hcd-uhci.c | 8 - hw/virtio/virtio-pci.c | 3 -- hw/watchdog/wdt_i6300esb.c | 8 - hw/xen/xen_pt.c| 20 hw/xen/xen_pt_msi.c| 2 -- include/exec/memory.h | 12 +-- include/hw/scsi/scsi.h | 7 include/hw/sysbus.h| 1 - ioport.c | 11 +-- memory.c | 28 +--- qom/object.c | 16 +++-- 62 files changed, 183 insertions(+), 308 deletions(-) -- 1.8.3.1
[Qemu-devel] [PULL 03/20] scsi-block: extract scsi_block_is_passthrough
This will be used for both scsi_block_new_request and the scsi-block implementation of parse_cdb. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 38 ++ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index d47ecd6..81b7276 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2501,12 +2501,8 @@ static int scsi_block_initfn(SCSIDevice *dev) return scsi_initfn(&s->qdev); } -static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, - uint32_t lun, uint8_t *buf, - void *hba_private) +static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf) { -SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); - switch (buf[0]) { case READ_6: case READ_10: @@ -2523,9 +2519,9 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, case WRITE_VERIFY_12: case WRITE_VERIFY_16: /* If we are not using O_DIRECT, we might read stale data from the -* host cache if writes were made using other commands than these -* ones (such as WRITE SAME or EXTENDED COPY, etc.). So, without -* O_DIRECT everything must go through SG_IO. + * host cache if writes were made using other commands than these + * ones (such as WRITE SAME or EXTENDED COPY, etc.). So, without + * O_DIRECT everything must go through SG_IO. */ if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) { break; @@ -2542,13 +2538,31 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, * just make scsi-block operate the same as scsi-generic for them. */ if (s->qdev.type != TYPE_ROM) { -return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun, - hba_private); +return false; } +break; + +default: +break; } -return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun, - hba_private); +return true; +} + + +static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, + uint32_t lun, uint8_t *buf, + void *hba_private) +{ +SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); + +if (scsi_block_is_passthrough(s, buf)) { +return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun, + hba_private); +} else { +return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun, + hba_private); +} } #endif -- 1.8.3.1
[Qemu-devel] [PULL 05/20] virtio-scsi: implement parse_cdb
Enable passthrough of vendor-specific commands. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 25 + 1 file changed, 25 insertions(+) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 0eb069a..2dd9255 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -406,6 +406,30 @@ static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status, virtio_scsi_complete_cmd_req(req); } +static int virtio_scsi_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, + uint8_t *buf, void *hba_private) +{ +VirtIOSCSIReq *req = hba_private; + +if (cmd->len == 0) { +cmd->len = MIN(VIRTIO_SCSI_CDB_SIZE, SCSI_CMD_BUF_SIZE); +memcpy(cmd->buf, buf, cmd->len); +} + +/* Extract the direction and mode directly from the request, for + * host device passthrough. + */ +cmd->xfer = req->qsgl.size; +if (cmd->xfer == 0) { +cmd->mode = SCSI_XFER_NONE; +} else if (iov_size(req->elem.in_sg, req->elem.in_num) > req->resp_size) { +cmd->mode = SCSI_XFER_FROM_DEV; +} else { +cmd->mode = SCSI_XFER_TO_DEV; +} +return 0; +} + static QEMUSGList *virtio_scsi_get_sg_list(SCSIRequest *r) { VirtIOSCSIReq *req = r->hba_private; @@ -658,6 +682,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .change = virtio_scsi_change, .hotplug = virtio_scsi_hotplug, .hot_unplug = virtio_scsi_hot_unplug, +.parse_cdb = virtio_scsi_parse_cdb, .get_sg_list = virtio_scsi_get_sg_list, .save_request = virtio_scsi_save_request, .load_request = virtio_scsi_load_request, -- 1.8.3.1
[Qemu-devel] [PULL 02/20] scsi-bus: introduce parse_cdb in SCSIDeviceClass and SCSIBusInfo
These callbacks will let devices do their own request parsing, or defer it to the bus. If the bus does not provide an implementation, in turn, fall back to the default parsing routine. Swap the first two arguments to scsi_req_parse, and rename it to scsi_req_parse_cdb, for consistency. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 26 +++--- include/hw/scsi/scsi.h | 6 ++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index ca4e9f3..d97d18a 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -9,7 +9,7 @@ static char *scsibus_get_dev_path(DeviceState *dev); static char *scsibus_get_fw_dev_path(DeviceState *dev); -static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf); +static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf); static void scsi_req_dequeue(SCSIRequest *req); static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len); static void scsi_target_free_buf(SCSIRequest *req); @@ -54,6 +54,20 @@ static void scsi_device_destroy(SCSIDevice *s) } } +int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, + void *hba_private) +{ +SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus); +int rc; + +assert(cmd->len == 0); +rc = scsi_req_parse_cdb(dev, cmd, buf); +if (bus->info->parse_cdb) { +rc = bus->info->parse_cdb(dev, cmd, buf, hba_private); +} +return rc; +} + static SCSIRequest *scsi_device_alloc_req(SCSIDevice *s, uint32_t tag, uint32_t lun, uint8_t *buf, void *hba_private) { @@ -562,6 +576,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, { SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus); const SCSIReqOps *ops; +SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d); SCSIRequest *req; SCSICommand cmd = { .len = 0 }; int ret; @@ -587,7 +602,12 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, ops = NULL; } -ret = scsi_req_parse(&cmd, d, buf); +if (ops != NULL || !sc->parse_cdb) { +ret = scsi_req_parse_cdb(d, &cmd, buf); +} else { +ret = sc->parse_cdb(d, &cmd, buf, hba_private); +} + if (ret != 0) { trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]); req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private); @@ -1190,7 +1210,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) return lba; } -static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) { int rc; diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 1adb549..4a0b860 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -76,6 +76,8 @@ typedef struct SCSIDeviceClass { DeviceClass parent_class; int (*init)(SCSIDevice *dev); void (*destroy)(SCSIDevice *s); +int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, + void *hba_private); SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun, uint8_t *buf, void *hba_private); void (*unit_attention_reported)(SCSIDevice *s); @@ -131,6 +133,8 @@ struct SCSIReqOps { struct SCSIBusInfo { int tcq; int max_channel, max_target, max_lun; +int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, + void *hba_private); void (*transfer_data)(SCSIRequest *req, uint32_t arg); void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid); void (*cancel)(SCSIRequest *req); @@ -244,6 +248,8 @@ void scsi_req_free(SCSIRequest *req); SCSIRequest *scsi_req_ref(SCSIRequest *req); void scsi_req_unref(SCSIRequest *req); +int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, + void *hba_private); void scsi_req_build_sense(SCSIRequest *req, SCSISense sense); void scsi_req_print(SCSIRequest *req); void scsi_req_continue(SCSIRequest *req); -- 1.8.3.1
[Qemu-devel] [PULL 01/20] scsi-bus: prepare scsi_req_new for introduction of parse_cdb
The per-SCSIDevice parse_cdb callback must not be called if the request will go through special SCSIReqOps, so detect the special cases early enough. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 51 ++- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 4341754..ca4e9f3 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -561,13 +561,38 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, uint8_t *buf, void *hba_private) { SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus); +const SCSIReqOps *ops; SCSIRequest *req; -SCSICommand cmd; +SCSICommand cmd = { .len = 0 }; +int ret; + +if ((d->unit_attention.key == UNIT_ATTENTION || + bus->unit_attention.key == UNIT_ATTENTION) && +(buf[0] != INQUIRY && + buf[0] != REPORT_LUNS && + buf[0] != GET_CONFIGURATION && + buf[0] != GET_EVENT_STATUS_NOTIFICATION && + + /* + * If we already have a pending unit attention condition, + * report this one before triggering another one. + */ + !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) { +ops = &reqops_unit_attention; +} else if (lun != d->lun || + buf[0] == REPORT_LUNS || + (buf[0] == REQUEST_SENSE && d->sense_len)) { +ops = &reqops_target_command; +} else { +ops = NULL; +} -if (scsi_req_parse(&cmd, d, buf) != 0) { +ret = scsi_req_parse(&cmd, d, buf); +if (ret != 0) { trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]); req = scsi_req_alloc(&reqops_invalid_opcode, d, tag, lun, hba_private); } else { +assert(cmd.len != 0); trace_scsi_req_parsed(d->id, lun, tag, buf[0], cmd.mode, cmd.xfer); if (cmd.lba != -1) { @@ -577,25 +602,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, if (cmd.xfer > INT32_MAX) { req = scsi_req_alloc(&reqops_invalid_field, d, tag, lun, hba_private); -} else if ((d->unit_attention.key == UNIT_ATTENTION || - bus->unit_attention.key == UNIT_ATTENTION) && - (buf[0] != INQUIRY && - buf[0] != REPORT_LUNS && - buf[0] != GET_CONFIGURATION && - buf[0] != GET_EVENT_STATUS_NOTIFICATION && - - /* -* If we already have a pending unit attention condition, -* report this one before triggering another one. -*/ - !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) { -req = scsi_req_alloc(&reqops_unit_attention, d, tag, lun, - hba_private); -} else if (lun != d->lun || - buf[0] == REPORT_LUNS || - (buf[0] == REQUEST_SENSE && d->sense_len)) { -req = scsi_req_alloc(&reqops_target_command, d, tag, lun, - hba_private); +} else if (ops) { +req = scsi_req_alloc(ops, d, tag, lun, hba_private); } else { req = scsi_device_alloc_req(d, tag, lun, buf, hba_private); } @@ -1186,6 +1194,7 @@ static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) { int rc; +cmd->lba = -1; switch (buf[0] >> 5) { case 0: cmd->len = 6; -- 1.8.3.1
[Qemu-devel] [PULL 06/20] qom: object: delete properties before calling instance_finalize
This ensures that the children's unparent callback will still have a usable parent. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- qom/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qom/object.c b/qom/object.c index 0e8267b..f301bc2 100644 --- a/qom/object.c +++ b/qom/object.c @@ -418,8 +418,8 @@ static void object_finalize(void *data) Object *obj = data; TypeImpl *ti = obj->class->type; -object_deinit(obj, ti); object_property_del_all(obj); +object_deinit(obj, ti); g_assert(obj->ref == 0); if (obj->free) { -- 1.8.3.1
[Qemu-devel] [PULL 04/20] scsi-block, scsi-generic: implement parse_cdb
The callback lets the bus provide the direction and transfer count for passthrough commands, enabling passthrough of vendor-specific commands. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 3 +-- hw/scsi/scsi-disk.c| 14 ++ hw/scsi/scsi-generic.c | 7 +++ include/hw/scsi/scsi.h | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index d97d18a..6f4462b 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -9,7 +9,6 @@ static char *scsibus_get_dev_path(DeviceState *dev); static char *scsibus_get_fw_dev_path(DeviceState *dev); -static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf); static void scsi_req_dequeue(SCSIRequest *req); static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len); static void scsi_target_free_buf(SCSIRequest *req); @@ -1210,7 +1209,7 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) return lba; } -static int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) +int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf) { int rc; diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 81b7276..d55521d 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2564,6 +2564,19 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag, hba_private); } } + +static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd, + uint8_t *buf, void *hba_private) +{ +SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); + +if (scsi_block_is_passthrough(s, buf)) { +return scsi_bus_parse_cdb(&s->qdev, cmd, buf, hba_private); +} else { +return scsi_req_parse_cdb(&s->qdev, cmd, buf); +} +} + #endif #define DEFINE_SCSI_DISK_PROPERTIES()\ @@ -2672,6 +2685,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data) sc->init = scsi_block_initfn; sc->destroy = scsi_destroy; sc->alloc_req= scsi_block_new_request; +sc->parse_cdb= scsi_block_parse_cdb; dc->fw_name = "disk"; dc->desc = "SCSI block device passthrough"; dc->reset = scsi_disk_reset; diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 3733d2c..0b2ff90 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -490,6 +490,12 @@ static Property scsi_generic_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static int scsi_generic_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, + uint8_t *buf, void *hba_private) +{ +return scsi_bus_parse_cdb(dev, cmd, buf, hba_private); +} + static void scsi_generic_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -498,6 +504,7 @@ static void scsi_generic_class_initfn(ObjectClass *klass, void *data) sc->init = scsi_generic_initfn; sc->destroy = scsi_destroy; sc->alloc_req= scsi_new_request; +sc->parse_cdb= scsi_generic_parse_cdb; dc->fw_name = "disk"; dc->desc = "pass through generic scsi device (/dev/sg*)"; dc->reset = scsi_generic_reset; diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 4a0b860..a7a28e6 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -250,6 +250,7 @@ void scsi_req_unref(SCSIRequest *req); int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, void *hba_private); +int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf); void scsi_req_build_sense(SCSIRequest *req, SCSISense sense); void scsi_req_print(SCSIRequest *req); void scsi_req_continue(SCSIRequest *req); -- 1.8.3.1
[Qemu-devel] [PULL 13/20] memory: remove memory_region_destroy
The function is empty after the previous patch, so remove it. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- backends/hostmem.c | 10 -- hw/audio/ac97.c| 9 - hw/audio/es1370.c | 8 hw/audio/intel-hda.c | 1 - hw/block/nvme.c| 1 - hw/block/pflash_cfi01.c| 1 - hw/block/pflash_cfi02.c| 1 - hw/char/serial-pci.c | 3 --- hw/i386/kvm/pci-assign.c | 8 hw/ide/ahci.c | 2 -- hw/ide/cmd646.c| 5 - hw/ide/piix.c | 3 --- hw/ide/via.c | 3 --- hw/ipack/tpci200.c | 13 - hw/misc/ivshmem.c | 3 --- hw/misc/pci-testdev.c | 2 -- hw/misc/vfio.c | 7 --- hw/net/e1000.c | 2 -- hw/net/eepro100.c | 3 --- hw/net/ne2000.c| 1 - hw/net/pcnet-pci.c | 2 -- hw/net/rtl8139.c | 2 -- hw/net/stellaris_enet.c| 8 hw/net/vmxnet3.c | 4 hw/pci-bridge/pci_bridge_dev.c | 2 -- hw/pci/msix.c | 4 hw/pci/pci.c | 2 -- hw/pci/pci_bridge.c| 8 hw/pci/pcie_host.c | 1 - hw/pci/shpc.c | 1 - hw/scsi/esp-pci.c | 1 - hw/scsi/lsi53c895a.c | 10 -- hw/scsi/megasas.c | 3 --- hw/scsi/vmw_pvscsi.c | 3 --- hw/tpm/tpm_tis.c | 1 - hw/usb/hcd-uhci.c | 8 hw/virtio/virtio-pci.c | 3 --- hw/watchdog/wdt_i6300esb.c | 8 hw/xen/xen_pt.c| 20 hw/xen/xen_pt_msi.c| 2 -- include/exec/memory.h | 9 - memory.c | 5 - 42 files changed, 193 deletions(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index ca10c51..e7eec37 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -257,15 +257,6 @@ static void host_memory_backend_init(Object *obj) host_memory_backend_set_policy, NULL, NULL, NULL); } -static void host_memory_backend_finalize(Object *obj) -{ -HostMemoryBackend *backend = MEMORY_BACKEND(obj); - -if (memory_region_size(&backend->mr)) { -memory_region_destroy(&backend->mr); -} -} - MemoryRegion * host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp) { @@ -360,7 +351,6 @@ static const TypeInfo host_memory_backend_info = { .class_init = host_memory_backend_class_init, .instance_size = sizeof(HostMemoryBackend), .instance_init = host_memory_backend_init, -.instance_finalize = host_memory_backend_finalize, .interfaces = (InterfaceInfo[]) { { TYPE_USER_CREATABLE }, { } diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c index 45cb118..0e22bb9 100644 --- a/hw/audio/ac97.c +++ b/hw/audio/ac97.c @@ -1388,14 +1388,6 @@ static int ac97_initfn (PCIDevice *dev) return 0; } -static void ac97_exitfn (PCIDevice *dev) -{ -AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev); - -memory_region_destroy (&s->io_nam); -memory_region_destroy (&s->io_nabm); -} - static int ac97_init (PCIBus *bus) { pci_create_simple (bus, -1, "AC97"); @@ -1413,7 +1405,6 @@ static void ac97_class_init (ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS (klass); k->init = ac97_initfn; -k->exit = ac97_exitfn; k->vendor_id = PCI_VENDOR_ID_INTEL; k->device_id = PCI_DEVICE_ID_INTEL_82801AA_5; k->revision = 0x01; diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c index 5dbf803..e67d1ea 100644 --- a/hw/audio/es1370.c +++ b/hw/audio/es1370.c @@ -1042,13 +1042,6 @@ static int es1370_initfn (PCIDevice *dev) return 0; } -static void es1370_exitfn (PCIDevice *dev) -{ -ES1370State *s = DO_UPCAST (ES1370State, dev, dev); - -memory_region_destroy (&s->io); -} - static int es1370_init (PCIBus *bus) { pci_create_simple (bus, -1, "ES1370"); @@ -1061,7 +1054,6 @@ static void es1370_class_init (ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS (klass); k->init = es1370_initfn; -k->exit = es1370_exitfn; k->vendor_id = PCI_VENDOR_ID_ENSONIQ; k->device_id = PCI_DEVICE_ID_ENSONIQ_ES1370; k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO; diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index aa49b47..aba45fc 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -1155,7 +1155,6 @@ static void intel_hda_exit(PCIDevice *pci) IntelHDAState *d = INTEL_HDA(pci); msi_uninit(&d->pci); -memory_region_destroy(&d->mmio); } static int intel_hda_post_load(void *opaque, int version) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 5fd8f89..6d9a065 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -839,7 +83
[Qemu-devel] [PULL 07/20] qom: object: move unparenting to the child property's release callback
This ensures that the unparent callback is called automatically when the parent object is finalized. Note that there's no need to keep a reference neither in object_unparent nor in object_finalize_child_property. The reference held by the child property itself will do. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- qom/object.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/qom/object.c b/qom/object.c index f301bc2..1b00831 100644 --- a/qom/object.c +++ b/qom/object.c @@ -387,19 +387,9 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp) void object_unparent(Object *obj) { -if (!obj->parent) { -return; -} - -object_ref(obj); -if (obj->class->unparent) { -(obj->class->unparent)(obj); -} if (obj->parent) { object_property_del_child(obj->parent, obj, NULL); -obj->parent = NULL; } -object_unref(obj); } static void object_deinit(Object *obj, TypeImpl *type) @@ -1042,6 +1032,10 @@ static void object_finalize_child_property(Object *obj, const char *name, { Object *child = opaque; +if (child->class->unparent) { +(child->class->unparent)(child); +} +child->parent = NULL; object_unref(child); } -- 1.8.3.1
[Qemu-devel] [PULL 11/20] ioport: split deletion and destruction
Of the two functions portio_list_del and portio_list_destroy, the latter is just freeing a memory area. However, portio_list_del is the logical equivalent of memory_region_del_subregion so destruction of memory regions does not belong there. Actually, neither of these APIs are in use; portio is mostly used by ISA devices or VGAs, and neither of these is currently hot-unpluggable. Signed-off-by: Paolo Bonzini --- ioport.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ioport.c b/ioport.c index 3d91e79..dce94a3 100644 --- a/ioport.c +++ b/ioport.c @@ -149,6 +149,14 @@ void portio_list_set_flush_coalesced(PortioList *piolist) void portio_list_destroy(PortioList *piolist) { +MemoryRegionPortioList *mrpio; +unsigned i; + +for (i = 0; i < piolist->nr; ++i) { +mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr); +memory_region_destroy(&mrpio->mr); +g_free(mrpio); +} g_free(piolist->regions); } @@ -291,8 +299,5 @@ void portio_list_del(PortioList *piolist) for (i = 0; i < piolist->nr; ++i) { mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr); memory_region_del_subregion(piolist->address_space, &mrpio->mr); -memory_region_destroy(&mrpio->mr); -g_free(mrpio); -piolist->regions[i] = NULL; } } -- 1.8.3.1
[Qemu-devel] [PULL 10/20] nic: do not destroy memory regions in cleanup functions
The memory regions should be destroyed in the unrealize function; since these NICs are not even qdev-ified, they cannot be unplugged and they do not have to do anything to destroy their memory regions. Reviewed-by: Stefan Hajnoczi Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- hw/net/dp8393x.c | 3 --- hw/net/mcf_fec.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 789d385..7eab7ad 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -863,9 +863,6 @@ static void nic_cleanup(NetClientState *nc) { dp8393xState *s = qemu_get_nic_opaque(nc); -memory_region_del_subregion(s->address_space, &s->mmio); -memory_region_destroy(&s->mmio); - timer_del(s->watchdog); timer_free(s->watchdog); diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c index 4bff3de..22cd7cf 100644 --- a/hw/net/mcf_fec.c +++ b/hw/net/mcf_fec.c @@ -443,9 +443,6 @@ static void mcf_fec_cleanup(NetClientState *nc) { mcf_fec_state *s = qemu_get_nic_opaque(nc); -memory_region_del_subregion(s->sysmem, &s->iomem); -memory_region_destroy(&s->iomem); - g_free(s); } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 09/11] target-arm: Implement ARMv8 single-step handling for A64 code
On 19 August 2014 11:25, Peter Maydell wrote: > On 19 August 2014 10:56, Edgar E. Iglesias wrote: >> On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote: >>> --- a/target-arm/cpu.h >>> +++ b/target-arm/cpu.h >>> @@ -1211,6 +1211,10 @@ static inline bool arm_singlestep_active(CPUARMState >>> *env) >>> #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT) >>> #define ARM_TBFLAG_AA64_FPEN_SHIFT 2 >>> #define ARM_TBFLAG_AA64_FPEN_MASK (1 << ARM_TBFLAG_AA64_FPEN_SHIFT) >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3 >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << >>> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT) >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3 >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << >>> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT) >> >> Shouldn't these shifts/masks differ? > > Oops. Yes, they certainly should. The fix is just a simple s/3/4/ for the PSTATE_SS_SHIFT define. Does anybody want a retransmit of the series for this one-liner? thanks -- PMM
[Qemu-devel] [PULL 08/20] sysbus: remove unused function sysbus_del_io
Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- hw/core/sysbus.c| 5 - include/hw/sysbus.h | 1 - 2 files changed, 6 deletions(-) diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index f4e760d..414e2a1 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -242,11 +242,6 @@ void sysbus_add_io(SysBusDevice *dev, hwaddr addr, memory_region_add_subregion(get_system_io(), addr, mem); } -void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem) -{ -memory_region_del_subregion(get_system_io(), mem); -} - MemoryRegion *sysbus_address_space(SysBusDevice *dev) { return get_system_memory(); diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h index f5aaa05..0bb91a8 100644 --- a/include/hw/sysbus.h +++ b/include/hw/sysbus.h @@ -71,7 +71,6 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr, int priority); void sysbus_add_io(SysBusDevice *dev, hwaddr addr, MemoryRegion *mem); -void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem); MemoryRegion *sysbus_address_space(SysBusDevice *dev); /* Legacy helper function for creating devices. */ -- 1.8.3.1
[Qemu-devel] [PULL 12/20] memory: convert memory_region_destroy to object_unparent
Explicitly call object_unparent in the few places where we will re-create the memory region. If the memory region is simply being destroyed as part of device teardown, let QOM handle it. Signed-off-by: Paolo Bonzini --- docs/memory.txt | 15 ++- hw/display/vga.c | 2 +- hw/i386/kvmvapic.c| 2 +- hw/mips/gt64xxx_pci.c | 2 +- hw/misc/omap_gpmc.c | 2 +- hw/misc/vfio.c| 4 ++-- hw/ppc/ppc4xx_devs.c | 2 +- ioport.c | 2 +- memory.c | 1 - 9 files changed, 18 insertions(+), 14 deletions(-) diff --git a/docs/memory.txt b/docs/memory.txt index 5bdbdb3..b12f1f0 100644 --- a/docs/memory.txt +++ b/docs/memory.txt @@ -74,11 +74,16 @@ Region lifecycle A region is created by one of the constructor functions (memory_region_init*()) -and destroyed by the destructor (memory_region_destroy()). In between, -a region can be added to an address space by using memory_region_add_subregion() -and removed using memory_region_del_subregion(). Region attributes may be -changed at any point; they take effect once the region becomes exposed to the -guest. +and attached to an object. It is then destroyed by object_unparent() or simply +when the parent object dies. + +In between, a region can be added to an address space +by using memory_region_add_subregion() and removed using +memory_region_del_subregion(). Destroying the region implicitly +removes the region from the address space. + +Region attributes may be changed at any point; they take effect once +the region becomes exposed to the guest. Overlapping regions and priority diff --git a/hw/display/vga.c b/hw/display/vga.c index 68cfee2..65dab8d 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -176,7 +176,7 @@ static void vga_update_memory_access(VGACommonState *s) if (s->has_chain4_alias) { memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias); -memory_region_destroy(&s->chain4_alias); +object_unparent(OBJECT(&s->chain4_alias)); s->has_chain4_alias = false; s->plane_updated = 0xf; } diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index cb855c7..ee95963 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -584,7 +584,7 @@ static int vapic_map_rom_writable(VAPICROMState *s) if (s->rom_mapped_writable) { memory_region_del_subregion(as, &s->rom); -memory_region_destroy(&s->rom); +object_unparent(OBJECT(&s->rom)); } /* grab RAM memory region (region @rom_paddr may still be pc.rom) */ diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 22f63ce..1f2fe5f 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -297,7 +297,7 @@ static void gt64120_pci_mapping(GT64120State *s) if (s->PCI0IO_length) { memory_region_del_subregion(get_system_memory(), &s->PCI0IO_mem); - memory_region_destroy(&s->PCI0IO_mem); + object_unparent(OBJECT(&s->PCI0IO_mem)); } /* Map new IO address */ s->PCI0IO_start = s->regs[GT_PCI0IOLD] << 21; diff --git a/hw/misc/omap_gpmc.c b/hw/misc/omap_gpmc.c index cddea24..fbbe2ff 100644 --- a/hw/misc/omap_gpmc.c +++ b/hw/misc/omap_gpmc.c @@ -436,7 +436,7 @@ static void omap_gpmc_cs_unmap(struct omap_gpmc_s *s, int cs) } memory_region_del_subregion(get_system_memory(), &f->container); memory_region_del_subregion(&f->container, omap_gpmc_cs_memregion(s, cs)); -memory_region_destroy(&f->container); +object_unparent(OBJECT(&f->container)); } void omap_gpmc_reset(struct omap_gpmc_s *s) diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 0b9eba0..af9ae1f 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -2266,7 +2266,7 @@ static void vfio_vga_quirk_teardown(VFIODevice *vdev) while (!QLIST_EMPTY(&vdev->vga.region[i].quirks)) { VFIOQuirk *quirk = QLIST_FIRST(&vdev->vga.region[i].quirks); memory_region_del_subregion(&vdev->vga.region[i].mem, &quirk->mem); -memory_region_destroy(&quirk->mem); +object_unparent(OBJECT(&quirk->mem)); QLIST_REMOVE(quirk, next); g_free(quirk); } @@ -2290,7 +2290,7 @@ static void vfio_bar_quirk_teardown(VFIODevice *vdev, int nr) while (!QLIST_EMPTY(&bar->quirks)) { VFIOQuirk *quirk = QLIST_FIRST(&bar->quirks); memory_region_del_subregion(&bar->mem, &quirk->mem); -memory_region_destroy(&quirk->mem); +object_unparent(OBJECT(&quirk->mem)); QLIST_REMOVE(quirk, next); g_free(quirk); } diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c index 07f9d00..405bbe7 100644 --- a/hw/ppc/ppc4xx_devs.c +++ b/hw/ppc/ppc4xx_devs.c @@ -422,7 +422,7 @@ static void sdram_set_bcr(ppc4xx_sdram_t *sdram, &sdram->containers[n]); memory_region_del_subregion(&sdram->containers[n],
[Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name
From: Peter Crosthwaite Rather than having the name as separate state. This prepares support for creating a MemoryRegion dynamically (i.e. without memory_region_init() and friends) and the MemoryRegion still getting a usable name. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 1 - memory.c | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index d165b27..10f73d9 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -162,7 +162,6 @@ struct MemoryRegion { QTAILQ_HEAD(subregions, MemoryRegion) subregions; QTAILQ_ENTRY(MemoryRegion) subregions_link; QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced; -const char *name; uint8_t dirty_log_mask; unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; diff --git a/memory.c b/memory.c index b6b208f..8da29af 100644 --- a/memory.c +++ b/memory.c @@ -915,7 +915,6 @@ void memory_region_init(MemoryRegion *mr, if (size == UINT64_MAX) { mr->size = int128_2_64(); } -mr->name = g_strdup(name); if (name) { object_property_add_child_array(owner, name, OBJECT(mr)); @@ -1260,7 +1259,6 @@ static void memory_region_finalize(Object *obj) assert(memory_region_transaction_depth == 0); mr->destructor(mr); memory_region_clear_coalescing(mr); -g_free((char *)mr->name); g_free(mr->ioeventfds); } @@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr) const char *memory_region_name(const MemoryRegion *mr) { -return mr->name; +return object_get_canonical_path_component(OBJECT(mr)); } bool memory_region_is_ram(MemoryRegion *mr) -- 1.8.3.1
[Qemu-devel] [PULL 20/20] mtree: remove write-only field
ml->printed is never set to true. Signed-off-by: Paolo Bonzini --- memory.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/memory.c b/memory.c index 8da29af..031ff51 100644 --- a/memory.c +++ b/memory.c @@ -1972,7 +1972,6 @@ typedef struct MemoryRegionList MemoryRegionList; struct MemoryRegionList { const MemoryRegion *mr; -bool printed; QTAILQ_ENTRY(MemoryRegionList) queue; }; @@ -2002,7 +2001,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, /* check if the alias is already in the queue */ QTAILQ_FOREACH(ml, alias_print_queue, queue) { -if (ml->mr == mr->alias && !ml->printed) { +if (ml->mr == mr->alias) { found = true; } } @@ -2010,7 +2009,6 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, if (!found) { ml = g_new(MemoryRegionList, 1); ml->mr = mr->alias; -ml->printed = false; QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue); } mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx @@ -2092,10 +2090,8 @@ void mtree_info(fprintf_function mon_printf, void *f) mon_printf(f, "aliases\n"); /* print aliased regions */ QTAILQ_FOREACH(ml, &ml_head, queue) { -if (!ml->printed) { -mon_printf(f, "%s\n", memory_region_name(ml->mr)); -mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head); -} +mon_printf(f, "%s\n", memory_region_name(ml->mr)); +mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head); } QTAILQ_FOREACH_SAFE(ml, &ml_head, queue, ml2) { -- 1.8.3.1
[Qemu-devel] [PULL 09/20] vga: do not dynamically allocate chain4_alias
Instead, add a boolean variable to indicate the presence of the region. This avoids a repeated malloc/free (later we can also avoid the add_child/unparent by changing the offset/size of the alias). Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- hw/display/vga.c | 24 ++-- hw/display/vga_int.h | 3 ++- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 4b089a3..68cfee2 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -168,15 +168,18 @@ static uint8_t expand4to8[16]; static void vga_update_memory_access(VGACommonState *s) { -MemoryRegion *region, *old_region = s->chain4_alias; hwaddr base, offset, size; if (s->legacy_address_space == NULL) { return; } -s->chain4_alias = NULL; - +if (s->has_chain4_alias) { +memory_region_del_subregion(s->legacy_address_space, &s->chain4_alias); +memory_region_destroy(&s->chain4_alias); +s->has_chain4_alias = false; +s->plane_updated = 0xf; +} if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) == VGA_SR02_ALL_PLANES && s->sr[VGA_SEQ_MEMORY_MODE] & VGA_SR04_CHN_4M) { offset = 0; @@ -201,18 +204,11 @@ static void vga_update_memory_access(VGACommonState *s) break; } base += isa_mem_base; -region = g_malloc(sizeof(*region)); -memory_region_init_alias(region, memory_region_owner(&s->vram), +memory_region_init_alias(&s->chain4_alias, memory_region_owner(&s->vram), "vga.chain4", &s->vram, offset, size); memory_region_add_subregion_overlap(s->legacy_address_space, base, -region, 2); -s->chain4_alias = region; -} -if (old_region) { -memory_region_del_subregion(s->legacy_address_space, old_region); -memory_region_destroy(old_region); -g_free(old_region); -s->plane_updated = 0xf; +&s->chain4_alias, 2); +s->has_chain4_alias = true; } } @@ -1321,7 +1317,7 @@ static void vga_draw_text(VGACommonState *s, int full_update) s->font_offsets[1] = offset; full_update = 1; } -if (s->plane_updated & (1 << 2) || s->chain4_alias) { +if (s->plane_updated & (1 << 2) || s->has_chain4_alias) { /* if the plane 2 was modified since the last display, it indicates the font may have been modified */ s->plane_updated = 0; diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 5320abd..641f8f4 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -94,7 +94,8 @@ typedef struct VGACommonState { uint32_t vram_size; uint32_t vram_size_mb; /* property */ uint32_t latch; -MemoryRegion *chain4_alias; +bool has_chain4_alias; +MemoryRegion chain4_alias; uint8_t sr_index; uint8_t sr[256]; uint8_t gr_index; -- 1.8.3.1
[Qemu-devel] [PULL 14/20] tpm_tis: remove instance_finalize callback
It is never used, since ISA device are not hot-unpluggable. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- hw/tpm/tpm_tis.c | 8 1 file changed, 8 deletions(-) diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index d398c16..82747ee 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -896,13 +896,6 @@ static void tpm_tis_initfn(Object *obj) &s->mmio); } -static void tpm_tis_uninitfn(Object *obj) -{ -TPMState *s = TPM(obj); - -memory_region_del_subregion(get_system_memory(), &s->mmio); -} - static void tpm_tis_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -918,7 +911,6 @@ static const TypeInfo tpm_tis_info = { .parent = TYPE_ISA_DEVICE, .instance_size = sizeof(TPMState), .instance_init = tpm_tis_initfn, -.instance_finalize = tpm_tis_uninitfn, .class_init = tpm_tis_class_init, }; -- 1.8.3.1
[Qemu-devel] [PULL 15/20] loader: Abstract away ref to memory region names
From: Peter Crosthwaite Use the function provided rather than spying on the struct. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- hw/core/loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/core/loader.c b/hw/core/loader.c index 2bf6b8f..1a53f0f 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -955,7 +955,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict) if (rom->mr) { monitor_printf(mon, "%s" " size=0x%06zx name=\"%s\"\n", - rom->mr->name, + memory_region_name(rom->mr), rom->romsize, rom->name); } else if (!rom->fw_file) { -- 1.8.3.1
[Qemu-devel] [PULL 18/20] memory: Use memory_region_name for name access
From: Peter Crosthwaite Despite being local to memory.c, use the helper function. This prepares support for fully QOMifiying the name field of MR (which will remove this state from MR completely). Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- memory.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/memory.c b/memory.c index 1cde6ff..b6b208f 100644 --- a/memory.c +++ b/memory.c @@ -2027,8 +2027,8 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mr->romd_mode ? 'R' : '-', !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W' : '-', - mr->name, - mr->alias->name, + memory_region_name(mr), + memory_region_name(mr->alias), mr->alias_offset, mr->alias_offset + (int128_nz(mr->size) ? @@ -2046,7 +2046,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mr->romd_mode ? 'R' : '-', !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W' : '-', - mr->name); + memory_region_name(mr)); } QTAILQ_INIT(&submr_print_queue); @@ -2095,7 +2095,7 @@ void mtree_info(fprintf_function mon_printf, void *f) /* print aliased regions */ QTAILQ_FOREACH(ml, &ml_head, queue) { if (!ml->printed) { -mon_printf(f, "%s\n", ml->mr->name); +mon_printf(f, "%s\n", memory_region_name(ml->mr)); mtree_print_mr(mon_printf, f, ml->mr, 0, 0, &ml_head); } } -- 1.8.3.1
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3] spapr: Fix stale HTAB during live migration
On 19.08.14 08:17, Samuel Mendoza-Jonas wrote: > If a guest reboots during a running migration, changes to the > hash page table are not necessarily updated on the destination. > Opening a new file descriptor to the HTAB forces the migration > handler to resend the entire table. > > Signed-off-by: Samuel Mendoza-Jonas > --- > Changes in v3: Pointed out by David, htab_save_iterate could > potentially try to read before htab_fd is open again. > Leave opening the fd to the functions trying to read. > Changes in v2: Forgot check on kvmppc_get_htab_fd return value > hw/ppc/spapr.c | 25 + > include/hw/ppc/spapr.h | 1 + > 2 files changed, 26 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 3a6d26d..5b41318 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -997,6 +997,10 @@ static void spapr_reset_htab(sPAPREnvironment *spapr) > /* Kernel handles htab, we don't need to allocate one */ > spapr->htab_shift = shift; > kvmppc_kern_htab = true; > + > +/* Check if we are overlapping a migration */ > +if (spapr->htab_fd > 0) > +spapr->need_reset = true; A "need_reset" field in the sPAPR machine with semantics of "we triggered a reset, so if there's a migration in flight please consider the full HTAB as invalid and thus reopen the fd to fetch all entries again" is not exactly obvious to me. How about something like "htab_save_should_reopen_fd" or an even better name you might come up with :). Alternatively you could save the last time stamp of when the HTAB was allocated and compare to that. Alex
[Qemu-devel] [PULL 16/20] exec: Abstract away ref to memory region names
From: Peter Crosthwaite Use the function provided rather than spying on the struct. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index 5a2a25e..42688b6 100644 --- a/exec.c +++ b/exec.c @@ -1044,7 +1044,7 @@ static void *file_ram_alloc(RAMBlock *block, } /* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(block->mr->name); +sanitized_name = g_strdup(memory_region_name(block->mr)); for (c = sanitized_name; *c != '\0'; c++) { if (*c == '/') *c = '_'; @@ -1242,7 +1242,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block) new_block->host = phys_mem_alloc(new_block->length); if (!new_block->host) { fprintf(stderr, "Cannot set up guest memory '%s': %s\n", -new_block->mr->name, strerror(errno)); +memory_region_name(new_block->mr), strerror(errno)); exit(1); } memory_try_enable_merging(new_block->host, new_block->length); -- 1.8.3.1
[Qemu-devel] [PULL 17/20] memory: constify memory_region_name
From: Peter Crosthwaite It doesn't change the MR and some prospective call sites will have const MRs at hand. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 2 +- memory.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 5bd10d1..d165b27 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -511,7 +511,7 @@ void memory_region_unregister_iommu_notifier(Notifier *n); * * @mr: the memory region being queried */ -const char *memory_region_name(MemoryRegion *mr); +const char *memory_region_name(const MemoryRegion *mr); /** * memory_region_is_logging: return whether a memory region is logging writes diff --git a/memory.c b/memory.c index f11c035..1cde6ff 100644 --- a/memory.c +++ b/memory.c @@ -1308,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr) return int128_get64(mr->size); } -const char *memory_region_name(MemoryRegion *mr) +const char *memory_region_name(const MemoryRegion *mr) { return mr->name; } -- 1.8.3.1
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
"M.Kustova" writes: > On Tue, Aug 19, 2014 at 1:44 PM, Fam Zheng wrote: >> On Tue, 08/19 02:00, Maria Kustova wrote: [...] >>> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py >>> index fd97c40..b142577 100755 >>> --- a/tests/image-fuzzer/runner.py >>> +++ b/tests/image-fuzzer/runner.py [...] >>> @@ -279,29 +277,30 @@ class TestEnv(object): >>> "Backing file: %s\n" \ >>> % (self.seed, " ".join(current_cmd), >>>self.current_dir, backing_file_name) >>> - >>> temp_log = StringIO.StringIO() >>> try: >>> retcode = run_app(temp_log, current_cmd) >>> except OSError, e: >>> -multilog(test_summary + "Error: Start of '%s' failed. " \ >>> - "Reason: %s\n\n" % (os.path.basename( >>> - current_cmd[0]), e[1]), >>> +multilog(test_summary + >>> + ("Error: Start of '%s' failed. Reason: %s\n\n" >>> + % (os.path.basename(current_cmd[0]), e[1])), >> >> I prefer the old one. I don't like the special case in python syntax: '(val)' >> is just val, but '(val1, val2)' is a tuple. > > This 'tuple' format provides that '%' substitution will be applied > only to the string in the parentheses, > instead of an entire string (in this case including test summary). > The same rationale for cases below. What about something like multilog("%sError: Start of '%s' failed. Reason: %s\n\n" % (test_summary, os.path.basename(current_cmd[0]), e[1]), sys.stderr, self.log, self.parent_log) [...]
Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation
On 19.08.14 00:26, Joel Schopp wrote: > > On 08/18/2014 05:11 PM, Peter Maydell wrote: >> On 18 August 2014 22:54, Joel Schopp wrote: >>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque) >>> +{ >>> +PlatformDevtreeData *data = opaque; >>> +void *fdt = data->fdt; >>> +const char *parent_node = data->node; >>> +int compat_str_len; >>> +char *nodename; >>> +int i, ret; >>> +uint32_t *irq_attr; >>> +uint64_t *reg_attr; >>> +uint64_t mmio_base; >>> +uint64_t irq_number; >>> +gchar mmio_base_prop[8]; >>> +gchar irq_number_prop[8]; >>> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); >>> +VFIODevice *vbasedev = &vdev->vbasedev; >>> +Object *obj = OBJECT(sbdev); >>> + >>> +mmio_base = object_property_get_int(obj, "mmio[0]", NULL); >>> + >>> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node, >>> + vbasedev->name, >>> + mmio_base); >>> + >>> +qemu_fdt_add_subnode(fdt, nodename); >>> + >>> +compat_str_len = strlen(vdev->compat) + 1; >> At this point you've already substituted the NULs in, >> so you can't call strlen(), I think. >> >>> +qemu_fdt_setprop(fdt, nodename, "compatible", >>> +vdev->compat, compat_str_len); >>> + >>> +reg_attr = g_new(uint64_t, vbasedev->num_regions*4); >>> + >>> +for (i = 0; i < vbasedev->num_regions; i++) { >>> +snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i); >>> +mmio_base = object_property_get_int(obj, mmio_base_prop, NULL); >>> +reg_attr[2*i] = 1; >>> +reg_attr[2*i+1] = mmio_base; >>> +reg_attr[2*i+2] = 1; >>> +reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem); >>> +} >>> >>> This should be 4 instead of 2. >>> Also, to support 64 bit systems I think this should be 2 instead of 1. >> Actually it depends entirely on what the board has done to >> create the device tree node that we're inserting this child >> node into. For ARM boot.c sets both #address-cells and >> #size-cells to 2 regardless of whether the system is 32 >> or 64 bits, for simplicity. I imagine PPC does something >> different. If we're editing a dtb that the user passed in (which >> I think would be pretty lunatic so we shouldn't do this) >> we'd have to actually walk the dtb to try to figure out what >> the semantics of the reg property should be. > For the index [2*i],[2*i+1], etc is clearly a bug as when i = 1 it will > overwrite two of the values. Changing that to [4*i],[4*i+1],etc fixes it. > > I think you are right on the size. I also wonder if the user doesn't > pass in a dtb if qemu should try to recreate the device-tree entry from > the platform device entry in the host kernel? If so would that best be > done by recreating the values from /proc/device-tree ? > > I also wish that qemu had a flag to output the generated dtb to a file > much like lkvm (kvmtool) has. It does. "qemu-system-foo -machine dumpdtb=mydtb.dtb" should dump the generated dtb into a file called mydtb.dtb. Alex
Re: [Qemu-devel] [PATCH v3 0/4] block: Use g_new() & friends more
Am 19.08.2014 um 10:31 hat Markus Armbruster geschrieben: > PATCH 1+2 convert some allocations. While preparing them, I stumbled > over dead error handling and some useless casts, which led to PATCH > 3+4. > > I posted a tree-wide version of PATCH 1 some time ago, and was told to > split it up. This is the block part, redone from scratch. Other > parts available on request. > > v3: > * Manual style cleanups in PATCH 4 [Max, Jeff] > * PATCH 1-3 unchanged. > v2: > * Regenerated with spatch. > * Because of that, I decided not to carry Max's R-by forward. > > Markus Armbruster (4): > block: Use g_new() & friends where that makes obvious sense > block: Use g_new() & friends to avoid multiplying sizes > qemu-io-cmds: g_renew() can't fail, bury dead error handling > block: Drop some superfluous casts from void * Thanks, applied all to the block branch. Kevin
Re: [Qemu-devel] [PULL 0/3] QMP queue
On 18 August 2014 20:26, Luiz Capitulino wrote: > Three little birds. > > The following changes since commit 08ab59770da57648bfb8fc9be37f0ef7fb50b0f9: > > Merge remote-tracking branch 'remotes/mcayland/qemu-sparc' into staging > (2014-08-18 12:55:02 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp > > for you to fetch changes up to b3dd1b8c295636e64ceb14cdc4db6420d7319e38: > > monitor: fix use after free (2014-08-18 14:39:10 -0400) > > > Chen Gang (1): > dump.c: Fix memory leak issue in cleanup processing for dump_init() > > Hani Benhabiles (1): > monitor: Remove hardcoded watchdog event names > > Michael S. Tsirkin (1): > monitor: fix use after free > > dump.c| 18 +- > include/monitor/monitor.h | 2 +- > monitor.c | 19 ++- > stubs/fdset-remove-fd.c | 3 +-- > 4 files changed, 17 insertions(+), 25 deletions(-) > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH V2 0/2] runner: Control test duration
Am 19.08.2014 um 11:18 hat Stefan Hajnoczi geschrieben: > On Tue, Aug 19, 2014 at 12:02:33AM +0400, Maria Kustova wrote: > > The first patch adds the '--duration SECONDS' argument. After the specified > > duration the runner allows to end the current test and then exits. > > > > The second patch adds forced termination of a program under test, if the > > test > > execution takes more than 10 minutes to indicate program freezes. > > > > If a program under test hangs, then the specified test duration can be > > overrun > > up to 10 minutes. > > > > The patch series is based on https://github.com/stefanha/qemu/commits/block, > > commit 07a45925fa88376f8583a333e74f7eeb0f455685 > > > > v1 -> v2: > > * Trivial fixes based on the review of Fam Zheng > > * Increased time-out (in some cases 5 minutes interval returned false > >negatives) > > > > Maria Kustova (2): > > runner: Add an argument for test duration > > runner: Kill a program under test by time-out > > > > tests/image-fuzzer/runner.py | 50 > > +--- > > 1 file changed, 42 insertions(+), 8 deletions(-) > > Reviewed-by: Stefan Hajnoczi Thanks, applied all to the block branch. Kevin pgp4gYglQGwAP.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL
Am 18.08.2014 um 13:41 hat Michael Tokarev geschrieben: > Just log to stderr unconditionally, like other similar code does. > > Signed-off-by: Michael Tokarev > --- > block/vvfat.c |5 - > 1 file changed, 5 deletions(-) > > diff --git a/block/vvfat.c b/block/vvfat.c > index 70176b1..ea37023 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -1082,11 +1082,6 @@ static int vvfat_open(BlockDriverState *bs, QDict > *options, int flags, > vvv = s; > #endif > > -DLOG(if (stderr == NULL) { > -stderr = fopen("vvfat.log", "a"); > -setbuf(stderr, NULL); > -}) > - > opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > qemu_opts_absorb_qdict(opts, options, &local_err); > if (local_err) { Michael, it's fine to merge trivial block patches through your tree, but can you please keep Stefan and me CCed anyway? This specific patch isn't as trivial as it might look at the first sight (in other words: it's wrong). The part that you probably missed is that stderr isn't the real one when DEBUG is set: #undef stderr #define stderr STDERR FILE* stderr = NULL; So now you have a lot of fprintf(NULL, ...), which obviously makes qemu segfault as soon as you enable debugging for vvfat. Kevin
Re: [Qemu-devel] [PULL v2 00/23] linux-user updates
On 19 August 2014 09:32, wrote: > From: Riku Voipio > > The same as previous series, except the patch "make binfmt flag O require P" > has been dropped. > > The following changes since commit 142f4ac5d5e024670ef4725e8943702b027e4218: > > Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-2014-08-15' > into staging (2014-08-15 18:44:48 +0100) > > are available in the git repository at: > > git://git.linaro.org/people/riku.voipio/qemu.git linux-user-for-upstream > > for you to fetch changes up to 8349027af60e861fcc401c928eb76f9f418815d7: Hi. I'm afraid this doesn't build on my ARM board: /root/qemu/linux-user/syscall.c: In function 'do_open_by_handle_at': /root/qemu/linux-user/syscall.c:5475:16: error: 'p' may be used uninitialized in this function [-Werror=uninitialized] The compiler seems to be correct here -- we should be passing 'fh' to unlock_user(), not 'p' (which seems to be completely useless and should be deleted). This function and name_to_handle_at() also both seem to be missing the necessary guest-to-host byteswaps on the fields in struct file_handle. thanks -- PMM
Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL
On 19 August 2014 12:58, Kevin Wolf wrote: > This specific patch isn't as trivial as it might look at the first > sight (in other words: it's wrong). The part that you probably missed is > that stderr isn't the real one when DEBUG is set: > > #undef stderr > #define stderr STDERR > FILE* stderr = NULL; Yikes. Can we rip that out as well, then? -- PMM
Re: [Qemu-devel] [PATCH] block/vvfat.c: remove debugging code to reinit stderr if NULL
On 08/19/2014 05:58 AM, Kevin Wolf wrote: > Am 18.08.2014 um 13:41 hat Michael Tokarev geschrieben: >> Just log to stderr unconditionally, like other similar code does. >> >> >> -DLOG(if (stderr == NULL) { >> -stderr = fopen("vvfat.log", "a"); >> -setbuf(stderr, NULL); >> -}) >> - > > This specific patch isn't as trivial as it might look at the first > sight (in other words: it's wrong). The part that you probably missed is > that stderr isn't the real one when DEBUG is set: > > #undef stderr > #define stderr STDERR > FILE* stderr = NULL; Eeek, that's horrible. I'd rather see code doing freopen("vvfat.log", "a", stderr) than the current horrid mess of redefining stderr away from its normal meaning. -- 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 09/11] target-arm: Implement ARMv8 single-step handling for A64 code
On Tue, Aug 19, 2014 at 11:46:23AM +0100, Peter Maydell wrote: > On 19 August 2014 11:25, Peter Maydell wrote: > > On 19 August 2014 10:56, Edgar E. Iglesias wrote: > >> On Fri, Aug 08, 2014 at 01:18:12PM +0100, Peter Maydell wrote: > >>> --- a/target-arm/cpu.h > >>> +++ b/target-arm/cpu.h > >>> @@ -1211,6 +1211,10 @@ static inline bool > >>> arm_singlestep_active(CPUARMState *env) > >>> #define ARM_TBFLAG_AA64_EL_MASK (0x3 << ARM_TBFLAG_AA64_EL_SHIFT) > >>> #define ARM_TBFLAG_AA64_FPEN_SHIFT 2 > >>> #define ARM_TBFLAG_AA64_FPEN_MASK (1 << ARM_TBFLAG_AA64_FPEN_SHIFT) > >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT 3 > >>> +#define ARM_TBFLAG_AA64_SS_ACTIVE_MASK (1 << > >>> ARM_TBFLAG_AA64_SS_ACTIVE_SHIFT) > >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_SHIFT 3 > >>> +#define ARM_TBFLAG_AA64_PSTATE_SS_MASK (1 << > >>> ARM_TBFLAG_AA64_PSTATE_SS_SHIFT) > >> > >> Shouldn't these shifts/masks differ? > > > > Oops. Yes, they certainly should. > > The fix is just a simple s/3/4/ for the PSTATE_SS_SHIFT > define. Does anybody want a retransmit of the series for > this one-liner? Hi, AFAICT, the rest of the series looks good, RB on all. Reviewed-by: Edgar E. Iglesias No need to repost for me. Cheers, Edgar