Re: [Qemu-devel] [PATCH for-1.4 2/2] configure: Fix build with XFree
Am 05.02.2013 01:21, schrieb Richard Henderson: > The build is broken on ppc64-linux, possibly only with new binutils: > > ld: hw/lm32/../milkymist-tmu2.o: undefined reference to symbol 'XFree' > ld: note: 'XFree' is defined in DSO /lib64/libX11.so.6 so try \ > adding it to the linker command line > > So let's follow the linker's advice. > > Signed-off-by: Richard Henderson > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 0657b1a..8789324 100755 > --- a/configure > +++ b/configure > @@ -2388,7 +2388,7 @@ fi > ## > # opengl probe, used by milkymist-tmu2 > if test "$opengl" != "no" ; then > - opengl_libs="-lGL" > + opengl_libs="-lGL -lX11" >cat > $TMPC << EOF > #include > #include What about using pkg-config to probe for opengl and get the correct linker options? Then static linking would also be fixed (it needs more than -lX11): $ pkg-config --libs gl -lGL $ pkg-config --libs --static gl -lGL -lm -lXext -lX11 -lpthread -lxcb -lXau -lXdmcp (tested on a Debian GNU Linux host) Regards Stefan W.
Re: [Qemu-devel] [PATCH] check-qjson: More thorough testing of UTF-8 in strings
Blue Swirl writes:
> On Mon, Feb 4, 2013 at 5:19 PM, Markus Armbruster wrote:
>> Test cases are scraped from Markus Kuhn's UTF-8 decoder capability and
>> stress test at
>> http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
>
> There's no license. Can we use that?
It's been used widely, but I cc'ed the page's author anyway.
>> Unfortunately, both JSON parser and formatter misbehave right now.
>> This test expects current, incorrect results. They're all clearly
>> marked, and are to be replaced by correct ones as the bugs get fixed.
>> See comments in new utf8_string() for details.
>
> This clearly calls for another fuzz test.
>
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> tests/check-qjson.c | 625
>
>> 1 file changed, 625 insertions(+)
>>
>> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
>> index 32ffb43..4590b3a 100644
>> --- a/tests/check-qjson.c
>> +++ b/tests/check-qjson.c
[...
>> +/* 3.1.9 Sequence of all 64 possible continuation bytes */
>> +{
>> +
> "\"\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F\xA0\xA1\xA2\xA3\xA4\xA5\xA6\xA7\xA8\xA9\xAA\xAB\xAC\xAD\xAE\xAF\xB0\xB1\xB2\xB3\xB4\xB5\xB6\xB7\xB8\xB9\xBA\xBB\xBC\xBD\xBE\xBF\"",
>
> Please wrap the long lines here and below.
Okay.
[...]
Re: [Qemu-devel] [PATCH 18/19] hw: remove CPU dependencies
Il 04/02/2013 23:30, Peter Maydell ha scritto:
> On 4 February 2013 17:30, Paolo Bonzini wrote:
>> Some devices or headers are using poisoned identifiers.
>> For .c files, some are useless and we remove them.
>>
>> For headers, wrap the definitions in the headers with
>>
>
> Truncated sentence?
>
>> Signed-off-by: Paolo Bonzini
>
>> --- a/hw/pci/host-apb.c
>> +++ b/hw/pci/host-apb.c
>> @@ -92,7 +92,7 @@ static void apb_config_writel (void *opaque, hwaddr addr,
>> {
>> APBState *s = opaque;
>>
>> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
>> addr, val);
>> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
>> addr, val);
>>
>> switch (addr & 0x) {
>> case 0x30 ... 0x4f: /* DMA error registers */
>> @@ -201,7 +201,7 @@ static uint64_t apb_config_readl (void *opaque,
>> val = 0;
>> break;
>> }
>> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, val);
>> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, val);
>>
>> return val;
>> }
>> @@ -218,7 +218,7 @@ static void apb_pci_config_write(void *opaque, hwaddr
>> addr,
>> APBState *s = opaque;
>>
>> val = qemu_bswap_len(val, size);
>> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
>> addr, val);
>> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
>> addr, val);
>> pci_data_write(s->bus, addr, val, size);
>> }
>>
>> @@ -230,7 +230,7 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr
>> addr,
>>
>> ret = pci_data_read(s->bus, addr, size);
>> ret = qemu_bswap_len(ret, size);
>> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret);
>> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
>> return ret;
>> }
>
> This appears to just be fixing format string errors in debug : should
> be a separate patch.
Yes.
>> diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
>> index bb9a1dd..8798cbe 100644
>> --- a/include/hw/arm/exynos4210.h
>> +++ b/include/hw/arm/exynos4210.h
>> @@ -84,7 +84,10 @@ typedef struct Exynos4210Irq {
>> qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
>> } Exynos4210Irq;
>>
>> -typedef struct Exynos4210State {
>> +typedef struct Exynos4210State Exynos4210State;
>> +
>> +#ifdef NEED_CPU_H
>> +struct Exynos4210State {
>> ARMCPU *cpu[EXYNOS4210_NCPUS];
>> Exynos4210Irq irqs;
>> qemu_irq *irq_table;
>> @@ -98,16 +101,17 @@ typedef struct Exynos4210State {
>> MemoryRegion boot_secondary;
>> MemoryRegion bootreg_mem;
>> i2c_bus *i2c_if[EXYNOS4210_I2C_NUMBER];
>> -} Exynos4210State;
>> +};
>>
>> void exynos4210_write_secondary(ARMCPU *cpu,
>> const struct arm_boot_info *info);
>> +#endif
>
> etc
>
> There's a lot of extra ifdeffery being added here. That makes
> me wonder if it's really the right way to do this...
No, it's not. The right way is either to have a .h file for each
device, or to qdevify things so that devices do not need an _init
function in the .c file (instead it can be an inline in the .h file).
Again, it's just getting us out of the local optimum. But it's close to
the end of the series so that it can be easily dropped.
Paolo
[Qemu-devel] [PATCH 1/2] qemu/iovec: Don't assert if sbytes is zero
From: "Aneesh Kumar K.V"
Since these values can possibly be sent from guest (for hw/9pfs), do a sanity
check
on them. A 9p write request with 0 bytes caused qemu to abort without this patch
Signed-off-by: Aneesh Kumar K.V
---
util/iov.c |4
1 file changed, 4 insertions(+)
diff --git a/util/iov.c b/util/iov.c
index c0f5c56..fbe675d 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -304,6 +304,10 @@ void qemu_iovec_concat_iov(QEMUIOVector *dst,
{
int i;
size_t done;
+
+if (!sbytes) {
+return;
+}
assert(dst->nalloc != -1);
for (i = 0, done = 0; done < sbytes && i < src_cnt; i++) {
if (soffset < src_iov[i].iov_len) {
--
1.7.10
Re: [Qemu-devel] [PATCH v4 02/13] qdev: change first argument of qbus_create_inplace to void *
Il 25/01/2013 15:04, Andreas Färber ha scritto: > Am 25.01.2013 14:12, schrieb Paolo Bonzini: >> Make it clear that no BUS() macro is needed in the callers (in fact it >> wouldn't work because the object has not been initialized yet with the >> right class). >> >> Suggested-by: Andreas Faerber >> Signed-off-by: Paolo Bonzini > > Acked-by: Andreas Färber > > But we should adjust most current callers then: > > hw/ide/qdev.c:qbus_create_inplace(&idebus->qbus, TYPE_IDE_BUS, dev, > NULL); > > ..._inplace(idebus, ...) > > hw/intel-hda.c:qbus_create_inplace(&bus->qbus, TYPE_HDA_BUS, dev, NULL); > hw/ipack.c:qbus_create_inplace(&bus->qbus, TYPE_IPACK_BUS, parent, > name); > hw/pci/pci.c:qbus_create_inplace(&bus->qbus, TYPE_PCI_BUS, parent, > name); > hw/pci/pci_bridge.c:qbus_create_inplace(&sec_bus->qbus, > TYPE_PCI_BUS, &dev->qdev, > hw/s390-virtio-bus.c:qbus_create_inplace((BusState *)bus, > TYPE_VIRTIO_S390_BUS, qdev, NULL); > > ..._inplace(bus, ...) > > hw/s390x/event-facility.c: > qbus_create_inplace(&event_facility->sbus.qbus, > hw/scsi-bus.c:qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, > NULL); > hw/usb/bus.c:qbus_create_inplace(&bus->qbus, TYPE_USB_BUS, host, NULL); > hw/usb/dev-smartcard-reader.c:qbus_create_inplace(&s->bus.qbus, > TYPE_CCID_BUS, &dev->qdev, NULL); > hw/virtio-pci.c:qbus_create_inplace((BusState *)bus, > TYPE_VIRTIO_PCI_BUS, qdev, NULL); > hw/virtio-serial-bus.c:qbus_create_inplace(&vser->bus.qbus, > TYPE_VIRTIO_SERIAL_BUS, dev, NULL); > > Should I send a follow-up patch for that or do you want to squash it > into yours to show the utility of your change? Sorry, I missed this message. I'll send a follow-up for 1.5. Paolo
Re: [Qemu-devel] [PATCH 00/19] hw/ directory restructuring
Il 04/02/2013 22:23, Peter Maydell ha scritto: > On 4 February 2013 20:51, Paolo Bonzini wrote: >> Il 04/02/2013 21:33, Peter Maydell ha scritto: The $FOO device models should all be in hw/$FOO. But there are some parts that do not fit in a category >>> >>> Surely this is what hw/misc is for? >> >> It's mostly interrupt/GPIO/DMA/memory/cache controllers. I placed some >> of them are in hw/misc because they are in common-obj-y, but I don't >> like the idea of hw/misc very much. hw/misc is really for the one-offs >> such as the tmp105.c temperature controller. >> >> If you prefer with creating a bunch of hw/ subdirectories like hw/pic, >> hw/gpio, hw/dma I can certainly move more stuff out of hw/$TARGET. As >> long as there is consensus, it's fine. But I wanted to avoid >> proliferation of hw/$FOO directories having only 5-10 files. > > I don't care if devices go in hw/misc or hw/extremely-fine-grained-category > as long as they don't go in hw/$TARGET :-) hw/misc be it then. :) >> The final submission may not have an hw/misc at all, the odd device like >> tmp105.c can go in hw/i2c. >> >>> (Also a hw/machines might be a good idea.) >> >> I don't like it very much because it's not clear which targets are used >> for which machines. I want to move away from the hw/arm/../foo.o hack >> that is used now. > > Why should you care which machine models happen to have particular > CPUs? In future we may even have machine models which have two CPUs > of different architectures... I think in that case what we want is to use composition to make the machine model code as small as possible. And then leave the little bits of glue code that remain in hw/$TARGET (or target-$TARGET/boards). > By analogy with this, anything we have which is a self-contained > device should definitely not be in hw/$TARGET. Small bits of code > like arm_boot.c, OK (and perhaps machine models, though I'm > dubious there because I'd like to see us moving to a framework > where machine models are just another kind of device). But if > it's really a device and we've been able to model it as a qdev > device then it shouldn't be in hw/$TARGET. Ok. > I could live with a rule that says "no devices in hw/$TARGET unless > they're there for legacy reasons because they share a source file > with a machine model" [and ideally eventually pull any self-contained > devices out of those files]. That's a nice clear line for reviewing > new patches. Yes. > Why, for example, is arm_gic.c moved to hw/arm but hw/heathrow_pic.c > moved to hw/misc rather than hw/ppc ? Because arm_gic.c was already in hw/arm/Makefile.objs, while hw/heathrow_pic.c was in hw/Makefile.objs. This certainly can be refined. Thanks for the useful ideas! Paolo
[Qemu-devel] [PATCH 2/2] qemu/9p: Don't ignore error in fid clunk
From: "Aneesh Kumar K.V"
We use the clunk request to do the actual xattr operation. So don't
ignore the error value for fid clunk.
Security model "none" don't support posix acl. Without this patch
guest won't get EOPNOTSUPP error on setxattr("system.posix_acl_access")
Signed-off-by: Aneesh Kumar K.V
---
hw/9pfs/virtio-9p.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 0aaf0d2..f526467 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -327,7 +327,7 @@ static int free_fid(V9fsPDU *pdu, V9fsFidState *fidp)
return retval;
}
-static void put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
+static int put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
{
BUG_ON(!fidp->ref);
fidp->ref--;
@@ -348,8 +348,9 @@ static void put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
pdu->s->migration_blocker = NULL;
}
}
-free_fid(pdu, fidp);
+return free_fid(pdu, fidp);
}
+return 0;
}
static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
@@ -1537,9 +1538,10 @@ static void v9fs_clunk(void *opaque)
* free the fid.
*/
fidp->ref++;
-err = offset;
-
-put_fid(pdu, fidp);
+err = put_fid(pdu, fidp);
+if (!err) {
+err = offset;
+}
out_nofid:
complete_pdu(s, pdu, err);
}
--
1.7.10
[Qemu-devel] [PATCH v2] usb-ehci: Replace PORTSC macros with variables
From: Kuo-Jung Su
Replace PORTSC macros with variables which could then be
configured in ehci__class_init(...) to support some
non-standard EHCI controllers.
Signed-off-by: Kuo-Jung Su
Cc: Gerd Hoffmann
Cc: Andreas
Cc: Peter Crosthwaite
---
Changes for V2:
- add missing port init to usb_ehci_pci_initfn()
hw/usb/hcd-ehci-pci.c|2 ++
hw/usb/hcd-ehci-sysbus.c |6 ++
hw/usb/hcd-ehci.c| 21 +
hw/usb/hcd-ehci.h| 12 ++--
4 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 0eb7826..bd56347 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -67,6 +67,8 @@ static int usb_ehci_pci_initfn(PCIDevice *dev)
s->capsbase = 0x00;
s->opregbase = 0x20;
+s->portscbase = 0x44;
+s->portnr = NB_PORTS;
usb_ehci_initfn(s, DEVICE(dev));
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index b68a66a..126ddd8 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -40,6 +40,8 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
s->capsbase = sec->capsbase;
s->opregbase = sec->opregbase;
+s->portscbase = sec->portscbase;
+s->portnr = sec->portnr;
s->dma = &dma_context_memory;
usb_ehci_initfn(s, DEVICE(dev));
@@ -73,6 +75,8 @@ static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
sec->capsbase = 0x100;
sec->opregbase = 0x140;
+sec->portscbase = 0x44;
+sec->portnr = NB_PORTS;
}
static const TypeInfo ehci_xlnx_type_info = {
@@ -87,6 +91,8 @@ static void ehci_exynos4210_class_init(ObjectClass *oc, void
*data)
sec->capsbase = 0x0;
sec->opregbase = 0x10;
+sec->portscbase = 0x44;
+sec->portnr = NB_PORTS;
}
static const TypeInfo ehci_exynos4210_type_info = {
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 7040659..a3e869a 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -988,7 +988,7 @@ static uint64_t ehci_port_read(void *ptr, hwaddr addr,
uint32_t val;
val = s->portsc[addr >> 2];
-trace_usb_ehci_portsc_read(addr + PORTSC_BEGIN, addr >> 2, val);
+trace_usb_ehci_portsc_read(addr + s->portscbase, addr >> 2, val);
return val;
}
@@ -1029,7 +1029,7 @@ static void ehci_port_write(void *ptr, hwaddr addr,
uint32_t old = *portsc;
USBDevice *dev = s->ports[port].dev;
-trace_usb_ehci_portsc_write(addr + PORTSC_BEGIN, addr >> 2, val);
+trace_usb_ehci_portsc_write(addr + s->portscbase, addr >> 2, val);
/* Clear rwc bits */
*portsc &= ~(val & PORTSC_RWC_MASK);
@@ -1062,7 +1062,7 @@ static void ehci_port_write(void *ptr, hwaddr addr,
*portsc &= ~PORTSC_RO_MASK;
*portsc |= val;
-trace_usb_ehci_portsc_change(addr + PORTSC_BEGIN, addr >> 2, *portsc, old);
+trace_usb_ehci_portsc_change(addr + s->portscbase, addr >> 2, *portsc,
old);
}
static void ehci_opreg_write(void *ptr, hwaddr addr,
@@ -2510,7 +2510,7 @@ void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
s->caps[0x01] = 0x00;
s->caps[0x02] = 0x00;
s->caps[0x03] = 0x01;/* HC version */
-s->caps[0x04] = NB_PORTS;/* Number of downstream ports */
+s->caps[0x04] = s->portnr; /* Number of downstream ports */
s->caps[0x05] = 0x00;/* No companion ports at present */
s->caps[0x06] = 0x00;
s->caps[0x07] = 0x00;
@@ -2518,8 +2518,13 @@ void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
s->caps[0x0a] = 0x00;
s->caps[0x0b] = 0x00;
+if (s->portnr > NB_PORTS) {
+hw_error("hcd-ehci: Too many ports! Max. port number=%d\n", NB_PORTS);
+exit(1);
+}
+
usb_bus_new(&s->bus, &ehci_bus_ops, dev);
-for(i = 0; i < NB_PORTS; i++) {
+for (i = 0; i < s->portnr; i++) {
usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
USB_SPEED_MASK_HIGH);
s->ports[i].dev = 0;
@@ -2538,13 +2543,13 @@ void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
memory_region_init_io(&s->mem_caps, &ehci_mmio_caps_ops, s,
"capabilities", CAPA_SIZE);
memory_region_init_io(&s->mem_opreg, &ehci_mmio_opreg_ops, s,
- "operational", PORTSC_BEGIN);
+ "operational", s->portscbase);
memory_region_init_io(&s->mem_ports, &ehci_mmio_port_ops, s,
- "ports", PORTSC_END - PORTSC_BEGIN);
+ "ports", 4 * s->portnr);
memory_region_add_subregion(&s->mem, s->capsbase, &s->mem_caps);
memory_region_add_subregion(&s->mem, s->opregbase, &s->mem_opreg);
-memory_region_add_subregion(&s->mem, s->opregbase + PORTSC_BEGIN,
+memory_region_add_subregion(&s->mem, s->opregbase + s->portscbase,
&s->mem_ports);
}
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci
Re: [Qemu-devel] [PATCH v2 01/20] arm: add Faraday a36x SoC platform support
2013/2/1 Andreas Färber : > 嗨 國榮, > > Am 01.02.2013 09:57, schrieb Kuo-Jung Su: >> Thanks for the information, and sorry for the mess I've done. > > You don't need to apologize for every review comment you get. It's > meant for improvements of results, not personal. :) > Thanks. It's so kind of you for helping me building up the common sense about patch process. >> I'll one-by-one re-send all the patches. >> >> However because most of my patches are new files, >> should I send-out the patches with detail change log? >> >> For example: >> >> [PATCH] dumb timer >> ... [PATCH v2 0/2] dumb timer (Cover letter) >> [PATCH v2 1/2] dumb timer (The one in Patch V1) >> [PATCH v2 2/2] dumb timer: coding style update (Change log for V2) >> .. [PATCH v3 0/2] dumb timer (Cover letter) >>[PATCH v3 1/2] dumb timer (The merged file in Patch V1 & v2) >>[PATCH v3 2/2] dumb timer: bug fix (Change log for V3) > > No, no, no. What you should do is just something like: > > [PATCH v3 0/x] Add Faraday A36x SoC platform support > [PATCH v3 1/x] arm: Add Faraday A360 and A369 machines > [PATCH v3 2/x] faraday_a36x: Add FT... timer > ... > > * v3 cover letter contains a change log going back to v1. > * v3 is not a reply to v2 (no --in-reply-to). This aids a threaded mail > display for reviewing and avoids an old version getting reviewed or applied. > * 1+/x are replies to 0/x (usually automatically by git-send-email). > That helps keep the patches together and in the right order. > * Bug fixes of your own code do not go separate (only if you were fixing > existing code from qemu.git). There's no need to introduce bugs and then > to fix them. Thanks, now I understand what am I supposed to do. > * Adding a stub machine in 1/x has the advantage that the patch is much > smaller and easier to review than first adding all devices and then > adding a machine that uses all of them. And each device being added in > (1+n)/x can be tested (system not fully working of course). I.e., the > machine will grow in functionality patch by patch. Thanks, I'm now re-formating the patch set. > * Maybe you can order EHCI last due to the refactoring work involved? I plan to again work on FUSBH200 - EHCI later when the A36x patch set have been applied. > > To aid with the requested reordering and squashing of bug fixes into > patches, `git rebase -i` and `git checkout -p` may be of help to you. > 'git rebase' is simply too complicated to me. I'm now using 'git cherry-pick' and 'git branch' to re-format the patches. > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- Best wishes, Kuo-Jung Su
Re: [Qemu-devel] [PATCH 5/6] load_linux: report open kernel file & its size error
OK, will fix.
Thanks!
在 2013-02-04一的 18:20 +，Blue Swirl写道：
> On Mon, Feb 4, 2013 at 2:27 AM, liguang wrote:
> > Signed-off-by: liguang
> > ---
> > hw/pc.c | 14 +++---
> > 1 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 01d00f6..0ccd775 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -652,12 +652,20 @@ static void load_linux(void *fw_cfg,
> > char *vmode;
> >
> > /* Align to 16 bytes as a paranoia measure */
> > -cmdline_size = (strlen(kernel_cmdline)+16) & ~15;
> > +cmdline_size = (strlen(kernel_cmdline)+16) & ~0xf;
>
> Here 15 is 16 - 1, so 0xf seems out of place. I'd use QEMU_ALIGN_UP().
>
> If you touch the line, please add spaces around '+'.
>
> >
> > /* load the kernel header */
> > f = fopen(kernel_filename, "rb");
> > -if (!f || !(kernel_size = get_file_size(f)) ||
> > -fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
> > +if (!f) {
> > +fprintf(stderr, "can't open kernel image file\n");
>
> The error message from strerror(errno) would be interesting.
>
> > +exit(1);
> > +}
> > +kernel_size = get_file_size(f);
> > +if (kernel_size <= 0) {
> > +fprintf(stderr, "can't get size of kernel image file\n");
> > +exit(1);
> > +}
> > +if (fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
> > MIN(ARRAY_SIZE(header), kernel_size)) {
> > fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
> > kernel_filename, strerror(errno));
> > --
> > 1.7.2.5
> >
> >
Re: [Qemu-devel] [PATCH 3/7] qdev_prop_parse(): report errors via Error
On 02/04/13 18:27, Luiz Capitulino wrote:
> On Fri, 1 Feb 2013 18:38:15 +0100
> Laszlo Ersek wrote:
>
>>
>> Signed-off-by: Laszlo Ersek
>
> I usually split this kind of patch the following way:
>
> 1. add an Error ** argument to the function reporting the error. Callers
> are changed to pass NULL for the new argument
If the called function already switches to error_set() /
error_propagate() at this point, then standing at this patch in a
possible bisection, the error message is lost. (The caller doesn't print
it *yet*, the callee doesn't print it *any longer*.)
If, OTOH, the called function still prints the error message here, and
doesn't pass it up (only its signature has changed), then:
> 2. Handling of the new error is added for each caller in a different
> patch (it's OK to group callers when the error handling is the same)
at some point there will be callers that need the callee to pass up the
error, and other callers (the ones not yet converted) that want the
callee not to.
I can
- switch callee and all callers at once (including prototype and error
handling), or
- put up with losing some messages inside the series, or
- put up with duplicate messages inside the series, or
- switch callee and callers at once (only prototype, error handling
stays unchanged), then switch them again all at once (error handling).
>
> 3. Drop error code return value from the function taking an Error **
> argument
>
> More comments below.
>
>> ---
>> hw/qdev-properties.h |4 +++-
>> hw/qdev-monitor.c| 26 +-
>> hw/qdev-properties.c | 12
>> 3 files changed, 32 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
>> index ddcf774..e33f31b 100644
>> --- a/hw/qdev-properties.h
>> +++ b/hw/qdev-properties.h
>> @@ -2,6 +2,7 @@
>> #define QEMU_QDEV_PROPERTIES_H
>>
>> #include "qdev-core.h"
>> +#include "qapi/error.h"
>>
>> /*** qdev-properties.c ***/
>>
>> @@ -99,7 +100,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>>
>> /* Set properties between creation and init. */
>> void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
>> -int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
>> +int qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
>> +Error **errp);
>> void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
>> void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
>> void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t
>> value);
>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> index 32be5a2..691bab5 100644
>> --- a/hw/qdev-monitor.c
>> +++ b/hw/qdev-monitor.c
>> @@ -100,16 +100,23 @@ static void qdev_print_devinfo(ObjectClass *klass,
>> void *opaque)
>> error_printf("\n");
>> }
>>
>> +typedef struct {
>> +DeviceState *dev;
>> +Error **errp;
>> +} PropertyContext;
>> +
>> static int set_property(const char *name, const char *value, void *opaque)
>> {
>> -DeviceState *dev = opaque;
>> +PropertyContext *ctx = opaque;
>> +Error *err = NULL;
>>
>> if (strcmp(name, "driver") == 0)
>> return 0;
>> if (strcmp(name, "bus") == 0)
>> return 0;
>>
>> -if (qdev_prop_parse(dev, name, value) == -1) {
>> +if (qdev_prop_parse(ctx->dev, name, value, &err) == -1) {
>> +error_propagate(ctx->errp, err);
>> return -1;
>
> The return error can be dropped if it's only used to signal success/failure.
set_property() is called from qemu_opt_foreach() and must match the
qemu_opt_loopfunc prototype.
>
>> }
>> return 0;
>> @@ -415,6 +422,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>> const char *driver, *path, *id;
>> DeviceState *qdev;
>> BusState *bus;
>> +Error *local_err = NULL;
>>
>> driver = qemu_opt_get(opts, "driver");
>> if (!driver) {
>> @@ -477,10 +485,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>> if (id) {
>> qdev->id = id;
>> }
>> -if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>> -qdev_free(qdev);
>> -return NULL;
>> +
>> +{
>> +PropertyContext ctx = { .dev = qdev, .errp = &local_err };
>> +
>> +if (qemu_opt_foreach(opts, set_property, &ctx, 1) != 0) {
>> +qerror_report_err(local_err);
>> +error_free(local_err);
>> +qdev_free(qdev);
>> +return NULL;
>> +}
>> }
>> +
>> if (qdev->id) {
>> object_property_add_child(qdev_get_peripheral(), qdev->id,
>>OBJECT(qdev), NULL);
>> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
>> index a8a31f5..8e3d014 100644
>> --- a/hw/qdev-properties.c
>> +++ b/hw/qdev-properties.c
>> @@ -835,7 +835,8 @@ void error_set_from_qdev_prop_error(Error **errp, int
>> ret, DeviceState *dev,
>> }
>> }
>>
>>
[Qemu-devel] [PATCH for-1.4 2/2] configure: Fix build with XFree
The build is broken on ppc64-linux, possibly only with new binutils: ld: hw/lm32/../milkymist-tmu2.o: undefined reference to symbol 'XFree' ld: note: 'XFree' is defined in DSO /lib64/libX11.so.6 so try \ adding it to the linker command line So let's follow the linker's advice. Signed-off-by: Richard Henderson --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 0657b1a..8789324 100755 --- a/configure +++ b/configure @@ -2388,7 +2388,7 @@ fi ## # opengl probe, used by milkymist-tmu2 if test "$opengl" != "no" ; then - opengl_libs="-lGL" + opengl_libs="-lGL -lX11" cat > $TMPC << EOF #include #include -- 1.8.1
[Qemu-devel] [PATCH for-1.4 1/2] bswap: Fix width of swap in leul_to_cpu
The misnamed HOST_LONG_BITS is really HOST_POINTER_BITS. Here we're
explicitly using an unsigned long, rather than uintptr_t, so it is
more correct to select the swap size via ULONG_MAX.
Acked-by: Andreas Färber
Signed-off-by: Richard Henderson
---
include/qemu/bswap.h | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
This is one of many patches around that fixes the build on big-endian
hosts. And indeed is not the only one that should be applied, as there
are arguably several bugs involved.
r~
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index e6d4798..d3af35d 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -2,8 +2,8 @@
#define BSWAP_H
#include "config-host.h"
-
#include
+#include
#include "fpu/softfloat.h"
#ifdef CONFIG_MACHINE_BSWAP_H
@@ -458,7 +458,15 @@ static inline void cpu_to_32wu(uint32_t *p, uint32_t v)
static inline unsigned long leul_to_cpu(unsigned long v)
{
-return le_bswap(v, HOST_LONG_BITS);
+/* In order to break an include loop between here and
+ qemu-common.h, don't rely on HOST_LONG_BITS. */
+#if ULONG_MAX == UINT32_MAX
+return le_bswap(v, 32);
+#elif ULONG_MAX == UINT64_MAX
+return le_bswap(v, 64);
+#else
+# error Unknown sizeof long
+#endif
}
#undef le_bswap
--
1.8.1
Re: [Qemu-devel] [PATCH 0/7] propagate Errors to do_device_add()
On 02/04/13 19:01, Luiz Capitulino wrote: > On Fri, 1 Feb 2013 18:38:12 +0100 > Laszlo Ersek wrote: > >> This series is the first in converting do_device_add() to qapi. > > I've reviewed this, although it doesn't apply on master anymore and > then I was unable to try it. Thanks for the review, I'll rebase / split it up. The conflict happened with Paolo's b09995ae "qdev: drop extra references at creation time". Laszlo
Re: [Qemu-devel] [Qemu-ppc] [PATCH] Fix circular dependency for HOST_LONG_BITS qemu-common.h <-> bswap.h
On 2013-02-04 16:07, Peter Maydell wrote: On 4 February 2013 23:52, Richard Henderson wrote: On 2013-02-04 15:30, David Gibson wrote: Anthony, Richard, anyone? Please apply - qemu has now been build-broken on all big endian platforms for a month. I know. See also my bswap.h patch which also fixes the width of long vs uintptr_t. No one seems willing to pick these up... In both cases, the patch: * was sent out after the soft freeze First version was the day before. * doesn't have a "for-1.4" tag Yes, that one is my fault. * doesn't have a summary line that clearly says "fixes build failure" either * hasn't got a Reviewed-by: tag from anybody It's gotten Acked-by's though. r~
Re: [Qemu-devel] [Qemu-ppc] [PATCH] Fix circular dependency for HOST_LONG_BITS qemu-common.h <-> bswap.h
On 4 February 2013 23:52, Richard Henderson wrote: > On 2013-02-04 15:30, David Gibson wrote: >> Anthony, Richard, anyone? >> >> Please apply - qemu has now been build-broken on all big endian >> platforms for a month. > > > I know. See also my bswap.h patch which also fixes the width > of long vs uintptr_t. No one seems willing to pick these up... In both cases, the patch: * was sent out after the soft freeze * doesn't have a "for-1.4" tag * doesn't have a summary line that clearly says "fixes build failure" either * hasn't got a Reviewed-by: tag from anybody so I'm not terribly surprised they haven't got picked up. You could start by reviewing each others' patches :-) -- PMM
Re: [Qemu-devel] [PATCH 2/7] do_device_add(): look up "device" opts list with qemu_find_opts_err()
On 02/04/13 17:38, Luiz Capitulino wrote:
> On Fri, 1 Feb 2013 18:38:14 +0100
> Laszlo Ersek wrote:
>
>>
>> Signed-off-by: Laszlo Ersek
>> ---
>> hw/qdev-monitor.c |7 +--
>> 1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> index 3ec9e49..32be5a2 100644
>> --- a/hw/qdev-monitor.c
>> +++ b/hw/qdev-monitor.c
>> @@ -590,14 +590,17 @@ void do_info_qdm(Monitor *mon, const QDict *qdict)
>> int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> {
>> Error *local_err = NULL;
>> +QemuOptsList *list;
>> QemuOpts *opts;
>>
>> -opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict,
>> &local_err);
>> -if (error_is_set(&local_err)) {
>> +if ((list = qemu_find_opts_err("device", &local_err)) == NULL ||
>> +(opts = qemu_opts_from_qdict(list, qdict, &local_err)) == NULL) {
>
> I'm not against this change, but qemu_find_opts() should never fails, so
> I don't mind the current code either.
>
> Now, if you do change it, I'd recommend separating the checks above and
> using a more standard idiom for checking for errors:
>
> list = qemu_find_opts_err("device", &local_err);
> if (error_is_set(&local_err)) {
> /* handle error */
> }
>
> opts = qemu_opts_from_qdict(..., &local_err);
> if (error_is_set(&local_err)) {
> /* handle error */
> }
/* handle error */ is exactly the same for both checks (print it, free
it, bail out); I wanted to avoid duplicating that block. I'll redo it
without the assignments in the controlling expression but will keep the
handler block common if you don't mind.
Thanks!
Laszlo
Re: [Qemu-devel] [Qemu-ppc] [PATCH] Fix circular dependency for HOST_LONG_BITS qemu-common.h <-> bswap.h
On 2013-02-04 15:30, David Gibson wrote: Anthony, Richard, anyone? Please apply - qemu has now been build-broken on all big endian platforms for a month. I know. See also my bswap.h patch which also fixes the width of long vs uintptr_t. No one seems willing to pick these up... r~
Re: [Qemu-devel] [PULL] virtio,make,pci,e1000,vfio,piix
Pulled. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH V4 00/22] Multiqueue virtio-net
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4?] isa: QOM'ify isa_bus_from_device()
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4] configure: Keep -Werror enabled for Release Candidates
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4] libqtest: Wait for the right child PID after killing QEMU
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 0/3] accel:some cleanup work for vm accelerator
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [Qemu-ppc] [PATCH] Fix circular dependency for HOST_LONG_BITS qemu-common.h <-> bswap.h
Anthony, Richard, anyone?
Please apply - qemu has now been build-broken on all big endian
platforms for a month.
On Tue, Jan 22, 2013 at 04:33:26PM +1100, David Gibson wrote:
> Commit c732a52d3e3b7ed42d7daa94ba40a83408cd6f22 from Richard Henderson
> changed leul_to_cpu() in bswap.h from a macro to an inline function. Both
> versions use HOST_LONG_BITS, but as an inline, HOST_LONG_BITS now needs to
> be evaluated at the point of definition rather than only when the macro is
> invoked.
>
> HOST_LONG_BITS is defined in qemu-common.h... which in turn includes
> bswap.h leading to a circular dependency. This doesn't show up on little
> endian hosts like x86, because the macros used within leul_to_cpu() end
> up removing the reference to HOST_LONG_BITS. This problem, however, breaks
> build on all big endian hosts such as powerpc.
>
> This patch fixes the problem by moving the basic HOST_LONG_BITS definition
> to osdep.h, which is already included before bswap.h.
>
> Cc: Richard Henderson
>
> Signed-off-by: David Gibson
> ---
> include/qemu-common.h |9 -
> include/qemu/osdep.h | 10 ++
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index ca464bb..ca7f8dc 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -72,15 +72,6 @@
> #define TIME_MAX LONG_MAX
> #endif
>
> -/* HOST_LONG_BITS is the size of a native pointer in bits. */
> -#if UINTPTR_MAX == UINT32_MAX
> -# define HOST_LONG_BITS 32
> -#elif UINTPTR_MAX == UINT64_MAX
> -# define HOST_LONG_BITS 64
> -#else
> -# error Unknown pointer size
> -#endif
> -
> #ifndef CONFIG_IOVEC
> #define CONFIG_IOVEC
> struct iovec {
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 87d3b9c..ebac074 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -3,6 +3,7 @@
>
> #include
> #include
> +#include
> #include
> #ifdef __OpenBSD__
> #include
> @@ -18,6 +19,15 @@ typedef unsigned intuint_fast16_t;
> typedef signed int int_fast16_t;
> #endif
>
> +/* HOST_LONG_BITS is the size of a native pointer in bits. */
> +#if UINTPTR_MAX == UINT32_MAX
> +# define HOST_LONG_BITS 32
> +#elif UINTPTR_MAX == UINT64_MAX
> +# define HOST_LONG_BITS 64
> +#else
> +# error Unknown pointer size
> +#endif
> +
> #ifndef glue
> #define xglue(x, y) x ## y
> #define glue(x, y) xglue(x, y)
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH for-1.4 v2] target-ppc: Fix target_ulong vs. hwaddr format mismatches
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [Qemu-stable] Patch queue for qemu-1.1.3 stable release
On Mon, Feb 4, 2013 at 4:40 AM, Michael Tokarev wrote: > Hello. > > This is my attempt at releasing a next stable/bugfix release of > abandomed 1.1 series of qemu. I tried my best to colledt all > fixes, and the last 1.1 has been out for quite a while now, so > it think it is time to release 1.1.3 or not do it at all. > > Several distributions are using 1.1 in their stable branches. I > know at least Debian (I maintain qemu package there these days) > and Gentoo uses it, maybe also Ubuntu. > Gentoo actually uses 1.2, I plan on publishing a similar patch queue that you did shortly for what would amount to a 1.2.3 release as well. But I'll gladly test your branch and provide some feedback when possible. -- Doug Goldstein
Re: [Qemu-devel] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH V4 RESEND 00/22] Multiqueue virtio-net
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1/2] tap: unbreak -netdev tap,fd=X
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4] acpi_piix4: fix segfault migrating from 1.2
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 00/10] -numa option parsing fixes & improvements
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 0/8] -numa option parsing fixes (v2)
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4 v4 00/12] qdev: correct reference counting
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4?] i2c: Drop I2C_SLAVE_FROM_QDEV() macro
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] sparc: disable qtest in make check
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 0/8] -numa option parsing fixes (v3)
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] xen: fix build problem introduced from per-queue peers
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Update version for 1.4.0-rc0
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] s390x: silence warning from GCC on uninitialized values
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for 1.4] target-s390x: Fix wrong comparison in interrupt handling
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH for-1.4 0/8] -numa option parsing fixes (v7)
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH V20 6/8] Add support for cancelling of a TPM command
On 02/04/2013 05:06 PM, Corey Bryant wrote: On 02/04/2013 01:48 PM, Stefan Berger wrote: On 02/04/2013 11:02 AM, Corey Bryant wrote: +/* + * As of Linux 3.7 the tpm_tis driver does not properly cancel + * commands for all TPMs. Any idea what the plan is for this issue? Is it an ongoing matter of adding kernel support as unsupported TPMs are identified? I submitted a patch to the tpmdd-devel mailing list fixing the cancellation issue with the particular TPM I have on one of my machines. Kent Yoder modified it to add a fix for yet another TPM (from a different vendor). https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37 The spec seems not be clear enough as to what bits must be set in the status register when a TPM command is canceled so that the so far implemented cancellation detection is insufficient and needs to be adapted to TPM vendors' implementations. All TIS interfaces are supposed to support cancellation, though. It sounds like this is a work in progress at the spec and Linux kernel level. It's certainly not ideal, but I think as long as a message is issued for unsupported TPMs, it shouldn't hold up integration of this support into QEMU. There may be a misunderstanding here: whether canceling an ongoing TPM command works is not something that QEMU can tell you. We will always find the cancel sysfs entry and we can write a byte into it to trigger the cancellation but whether the driver then detects the cancellation correctly is something different - the problem here is that the thread is waiting inside the driver for the command to complete and periodically check whether it has finished and/or was canceled. I previously used a different machine where cancellation was working but now with the new machine and a different vendor's TPM the cancel sysfs entry cancels the command fine but the driver does not detect the fact that the command was actually canceled and just continues waiting for the command's completion. Also since we don't have access to all TPM vendors' TPMs we cannot tell for sure which ones are working and which ones are not. Stefan
Re: [Qemu-devel] [PATCH 18/19] hw: remove CPU dependencies
On 4 February 2013 17:30, Paolo Bonzini wrote:
> Some devices or headers are using poisoned identifiers.
> For .c files, some are useless and we remove them.
>
> For headers, wrap the definitions in the headers with
>
Truncated sentence?
> Signed-off-by: Paolo Bonzini
> --- a/hw/pci/host-apb.c
> +++ b/hw/pci/host-apb.c
> @@ -92,7 +92,7 @@ static void apb_config_writel (void *opaque, hwaddr addr,
> {
> APBState *s = opaque;
>
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
> addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
> addr, val);
>
> switch (addr & 0x) {
> case 0x30 ... 0x4f: /* DMA error registers */
> @@ -201,7 +201,7 @@ static uint64_t apb_config_readl (void *opaque,
> val = 0;
> break;
> }
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, val);
>
> return val;
> }
> @@ -218,7 +218,7 @@ static void apb_pci_config_write(void *opaque, hwaddr
> addr,
> APBState *s = opaque;
>
> val = qemu_bswap_len(val, size);
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
> addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
> addr, val);
> pci_data_write(s->bus, addr, val, size);
> }
>
> @@ -230,7 +230,7 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr
> addr,
>
> ret = pci_data_read(s->bus, addr, size);
> ret = qemu_bswap_len(ret, size);
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
> return ret;
> }
This appears to just be fixing format string errors in debug : should
be a separate patch.
> diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
> index bb9a1dd..8798cbe 100644
> --- a/include/hw/arm/exynos4210.h
> +++ b/include/hw/arm/exynos4210.h
> @@ -84,7 +84,10 @@ typedef struct Exynos4210Irq {
> qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
> } Exynos4210Irq;
>
> -typedef struct Exynos4210State {
> +typedef struct Exynos4210State Exynos4210State;
> +
> +#ifdef NEED_CPU_H
> +struct Exynos4210State {
> ARMCPU *cpu[EXYNOS4210_NCPUS];
> Exynos4210Irq irqs;
> qemu_irq *irq_table;
> @@ -98,16 +101,17 @@ typedef struct Exynos4210State {
> MemoryRegion boot_secondary;
> MemoryRegion bootreg_mem;
> i2c_bus *i2c_if[EXYNOS4210_I2C_NUMBER];
> -} Exynos4210State;
> +};
>
> void exynos4210_write_secondary(ARMCPU *cpu,
> const struct arm_boot_info *info);
> +#endif
etc
There's a lot of extra ifdeffery being added here. That makes
me wonder if it's really the right way to do this...
-- PMM
Re: [Qemu-devel] [PATCH V20 6/8] Add support for cancelling of a TPM command
On 02/04/2013 01:48 PM, Stefan Berger wrote:
On 02/04/2013 11:02 AM, Corey Bryant wrote:
@@ -221,7 +243,24 @@ static void
tpm_passthrough_deliver_request(TPMBackend *tb)
static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
{
-/* cancelling an ongoing command is known not to work with some
TPMs */
+TPMPassthruState *tpm_pt = tb->s.tpm_pt;
+int n;
+
+/*
+ * As of Linux 3.7 the tpm_tis driver does not properly cancel
+ * commands for all TPMs.
Any idea what the plan is for this issue? Is it an ongoing matter of
adding kernel support as unsupported TPMs are identified?
I submitted a patch to the tpmdd-devel mailing list fixing the
cancellation issue with the particular TPM I have on one of my machines.
Kent Yoder modified it to add a fix for yet another TPM (from a
different vendor).
https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37
The spec seems not be clear enough as to what bits must be set in the
status register when a TPM command is canceled so that the so far
implemented cancellation detection is insufficient and needs to be
adapted to TPM vendors' implementations. All TIS interfaces are supposed
to support cancellation, though.
It sounds like this is a work in progress at the spec and Linux kernel
level. It's certainly not ideal, but I think as long as a message is
issued for unsupported TPMs, it shouldn't hold up integration of this
support into QEMU.
+ * Only cancel if we're busy so we don't cancel someone else's
+ * command, e.g., a command executed on the host.
+ */
+if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
+n = write(tpm_pt->cancel_fd, "-", 1);
+if (n != 1) {
+error_report("Canceling TPM command failed: %s\n",
+ strerror(errno));
+} else {
+tpm_pt->tpm_op_canceled = true;
+}
+}
Would an informational message make sense here for unsupported TPMs
(when tpm_pt->cancel_fd < 0)?
Sure, I can add that.
Great, thanks.
+snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device",
i);
+if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
+continue;
+}
+fd = open(path, O_WRONLY);
+break;
+}
+
+exit:
+if (fd >= 0) {
+tb->cancel_path = g_strdup(path);
+}
+
+return fd;
+}
+
A general question about the function above. I see that
"/sys/class/misc/tpm%u/device" will be explained in kernel
documentation here:
https://github.com/shpedoikal/linux/commit/4e21d66c9efbe740b5655bcf66e39873e354f8e9
And the following paths apparently have the same behavior. But are
these paths defined somewhere else?
"/sys/devices/pnp%u/%s"
"/sys/devices/platform/tpm_tis"
Also I think a comment pointing to documentation would be a
worth-while addition to the code.
The /sys/class/misc/tpm%u/device path seems to be always there
independent of what type of device the TPM is registered as (pnp or
platform). So I'll modify above code to always use that path instead.
Great, that would simplify things a bit.
Stefan
--
Regards,
Corey Bryant
Re: [Qemu-devel] [PATCH 4/7] block/vpc: Improve vpc_open (optimisation and error handling)
On Fri, Feb 01, 2013 at 10:51:31PM +0100, Stefan Weil wrote:
> * Always read the footer at the end of the image.
> The footer exists for all kinds of VHD images, so there is no need
> to read its copy at the start of dynamic VHD images.
>
> * Return error codes from bdrv_pread like in other block drivers.
>
> * Return -EMEDIUMTYPE for wrong medium (missing signature).
>
> * It's not necessary to return 0 on success, >= 0 is sufficient.
>
> Signed-off-by: Stefan Weil
> ---
> block/vpc.c | 46 ++
> 1 file changed, 22 insertions(+), 24 deletions(-)
>
Hi Stefan,
I was going to test against some VHD images I have, as well as run
qemu-io tests, but this patch series has conflicts with the current
master. Patches 1 & 3 have minor conflicts with commit 59294e4, but
this patch has pretty significant conflicts, so I didn't bother
resolving it to test.
Could you rebase, and then repost?
Thanks,
Jeff
> diff --git a/block/vpc.c b/block/vpc.c
> index bcc2ace..fff103b 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -164,34 +164,30 @@ static int vpc_open(BlockDriverState *bs, int flags)
> {
> BDRVVPCState *s = bs->opaque;
> int i;
> -struct vhd_footer *footer;
> +struct vhd_footer *footer = (struct vhd_footer *)s->footer_buf;
> struct vhd_dyndisk_header *dyndisk_header;
> uint8_t buf[FOOTER_SIZE];
> uint32_t checksum;
> -int err = -1;
> +int ret;
> int disk_type = VHD_DYNAMIC;
> +int64_t offset = bdrv_getlength(bs->file);
>
> -if (bdrv_pread(bs->file, 0, s->footer_buf, FOOTER_SIZE) != FOOTER_SIZE) {
> +if (offset < FOOTER_SIZE) {
> +ret = -EINVAL;
> goto fail;
> }
>
> -footer = (struct vhd_footer *)s->footer_buf;
> +/* The footer is always found at the end of the file. */
> +ret = bdrv_pread(bs->file, offset-FOOTER_SIZE, s->footer_buf,
> FOOTER_SIZE);
> +if (ret < 0) {
> +goto fail;
> +}
> if (strncmp(footer->creator, "conectix", 8)) {
> -int64_t offset = bdrv_getlength(bs->file);
> -if (offset < FOOTER_SIZE) {
> -goto fail;
> -}
> -/* If a fixed disk, the footer is found only at the end of the file
> */
> -if (bdrv_pread(bs->file, offset-FOOTER_SIZE, s->footer_buf,
> FOOTER_SIZE)
> -!= FOOTER_SIZE) {
> -goto fail;
> -}
> -if (strncmp(footer->creator, "conectix", 8)) {
> -goto fail;
> -}
> -disk_type = VHD_FIXED;
> +ret = -EMEDIUMTYPE;
> +goto fail;
> }
>
> +disk_type = be32_to_cpu(footer->type);
> checksum = be32_to_cpu(footer->checksum);
> footer->checksum = 0;
> if (vpc_checksum(s->footer_buf, FOOTER_SIZE) != checksum) {
> @@ -210,19 +206,21 @@ static int vpc_open(BlockDriverState *bs, int flags)
>
> /* Allow a maximum disk size of approximately 2 TB */
> if (bs->total_sectors >= 65535LL * 255 * 255) {
> -err = -EFBIG;
> +ret = -EFBIG;
> goto fail;
> }
>
> if (disk_type == VHD_DYNAMIC) {
> -if (bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
> -FOOTER_SIZE) != FOOTER_SIZE) {
> +ret = bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
> + FOOTER_SIZE);
> +if (ret < 0) {
> goto fail;
> }
>
> dyndisk_header = (struct vhd_dyndisk_header *) buf;
>
> if (strncmp(dyndisk_header->magic, "cxsparse", 8)) {
> +ret = -EINVAL;
> goto fail;
> }
>
> @@ -233,8 +231,9 @@ static int vpc_open(BlockDriverState *bs, int flags)
> s->pagetable = g_malloc(s->max_table_entries * 4);
>
> s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
> -if (bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> -s->max_table_entries * 4) != s->max_table_entries * 4) {
> +ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> + s->max_table_entries * 4);
> +if (ret < 0) {
> goto fail;
> }
>
> @@ -271,9 +270,8 @@ static int vpc_open(BlockDriverState *bs, int flags)
>"vpc", bs->device_name, "live migration");
> migrate_add_blocker(s->migration_blocker);
>
> -return 0;
> fail:
> -return err;
> +return ret;
> }
>
> static int vpc_reopen_prepare(BDRVReopenState *state,
> --
> 1.7.10.4
>
Re: [Qemu-devel] [PATCH] s390x: silence warning from GCC on uninitialized values
Am 04.02.2013 22:23, schrieb Anthony Liguori: > As best I can tell, this is a false positive. > > [aliguori@ccnode4 qemu-s390]$ make > CCs390x-softmmu/target-s390x/helper.o > /home/aliguori/git/qemu/target-s390x/helper.c: In function ‘do_interrupt’: > /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘addr’ may be > used uninitialized in this function [-Werror=maybe-uninitialized] > /home/aliguori/git/qemu/target-s390x/helper.c:620:20: note: ‘addr’ was > declared here > /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘mask’ may be > used uninitialized in this function [-Werror=maybe-uninitialized] > /home/aliguori/git/qemu/target-s390x/helper.c:620:14: note: ‘mask’ was > declared here > cc1: all warnings being treated as errors > make[1]: *** [target-s390x/helper.o] Error 1 > make: *** [subdir-s390x-softmmu] Error 2 > Yes, this is a false positive. A better compiler will complain when your patch was applied because addr, mask are assigned values which are never used... Would it be possible to completely eliminate variable "found" and move the DPRINTF, load_psw statements into the for loop (just before the break statement)? Stefan W.
Re: [Qemu-devel] [PATCH] linux-user: Support setgroups syscall with no groups
On 02/04/2013 02:07 PM, Peter Maydell wrote:
> On 4 February 2013 18:38, Eric Blake wrote:
>> On 02/02/2013 04:04 PM, dill...@dillona.com wrote:
>>> -
>>> -grouplist = alloca(gidsetsize * sizeof(gid_t));
>>> -target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize *
>>> 2, 1);
>>> -if (!target_grouplist) {
>>> -ret = -TARGET_EFAULT;
>>> -goto fail;
>>> +if (gidsetsize) {
>>> +grouplist = alloca(gidsetsize * sizeof(gid_t));
>>
>> Is this alloca() safe, or are you risking stack overflow if the user
>> passes an extremely large arg1?
>
> No, the linux-user has a number of long-standing not-terribly-safe
> alloca calls like this. If anybody wants to go through and fix them
> patches are welcome, but I don't think it's fair to require them
> to be fixed in order to get fairly simple patches like this in,
> where the patch is merely reindenting existing dubious code, not
> adding to the problem.
Point taken - the abuse of alloca() is pre-existing, so it shouldn't
block this particular patch.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] s390x: silence warning from GCC on uninitialized values
As best I can tell, this is a false positive.
[aliguori@ccnode4 qemu-s390]$ make
CCs390x-softmmu/target-s390x/helper.o
/home/aliguori/git/qemu/target-s390x/helper.c: In function ‘do_interrupt’:
/home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘addr’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
/home/aliguori/git/qemu/target-s390x/helper.c:620:20: note: ‘addr’ was
declared here
/home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘mask’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
/home/aliguori/git/qemu/target-s390x/helper.c:620:14: note: ‘mask’ was
declared here
cc1: all warnings being treated as errors
make[1]: *** [target-s390x/helper.o] Error 1
make: *** [subdir-s390x-softmmu] Error 2
Cc: Cornelia Huck
Cc: Stefan Weil
Signed-off-by: Anthony Liguori
---
target-s390x/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 8bd84ef..043feb2 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -617,7 +617,7 @@ static void do_ext_interrupt(CPUS390XState *env)
static void do_io_interrupt(CPUS390XState *env)
{
-uint64_t mask, addr;
+uint64_t mask = 0, addr = 0;
LowCore *lowcore;
IOIntQueue *q;
uint8_t isc;
--
1.8.0
Re: [Qemu-devel] [PATCH 00/19] hw/ directory restructuring
On 4 February 2013 20:51, Paolo Bonzini wrote: > Il 04/02/2013 21:33, Peter Maydell ha scritto: >>> The $FOO device models should all be in hw/$FOO. But there are some >>> parts that do not fit in a category >> >> Surely this is what hw/misc is for? > > It's mostly interrupt/GPIO/DMA/memory/cache controllers. I placed some > of them are in hw/misc because they are in common-obj-y, but I don't > like the idea of hw/misc very much. hw/misc is really for the one-offs > such as the tmp105.c temperature controller. > > If you prefer with creating a bunch of hw/ subdirectories like hw/pic, > hw/gpio, hw/dma I can certainly move more stuff out of hw/$TARGET. As > long as there is consensus, it's fine. But I wanted to avoid > proliferation of hw/$FOO directories having only 5-10 files. I don't care if devices go in hw/misc or hw/extremely-fine-grained-category as long as they don't go in hw/$TARGET :-) > The final submission may not have an hw/misc at all, the odd device like > tmp105.c can go in hw/i2c. > >> (Also a hw/machines might be a good idea.) > > I don't like it very much because it's not clear which targets are used > for which machines. I want to move away from the hw/arm/../foo.o hack > that is used now. Why should you care which machine models happen to have particular CPUs? In future we may even have machine models which have two CPUs of different architectures... >>> or are always target-specific, and >>> hw/$TARGET has them. >>> >>> Also, the kernel similarly has arch/$TARGET and drivers/$FOO. >> >> Yes, and the ARM kernel maintainers have just spent quite a lot >> of effort in cleaning things up to kick all the drivers out >> of arch/$TARGET and into drivers/$FOO where they belong. >> I'd rather we didn't get into that mess in the first place :-) > > Yes, and in fact I'm moving a lot of ARM devices away from hw/arm's > Makefile.objs to other directories (net/ for NICs, char/ for UARTs > etc.), and configuring them via default-configs/arm-softmmu.mak. > > But Linux still has a bunch of board-specific drivers in arch/arm/mach-* > that won't be moved to drivers/, and that's roughly what I left in > hw/$TARGET. (This is a bit of a tangent, but As I understand it, if it's really a driver it should be moving. >From https://lwn.net/Articles/443510/: "There will of course need to be at least some ARM-specific code [in arch/arm], but the end goal is for that code to be limited to ARM core architecture code and ARM SoC core architecture code. Furthermore, the ARM SoC core architecture code should consist primarily of small plugins for core-Linux-kernel frameworks". arch/arm64 won't have mach-* subdirs at all, we are stomping on that before it starts :-) By analogy with this, anything we have which is a self-contained device should definitely not be in hw/$TARGET. Small bits of code like arm_boot.c, OK (and perhaps machine models, though I'm dubious there because I'd like to see us moving to a framework where machine models are just another kind of device). But if it's really a device and we've been able to model it as a qdev device then it shouldn't be in hw/$TARGET. > (Note: some boards still have devices in hw/arm after these patches, > e.g. musicpal has a NIC and LCD controller. Those could indeed be > refactored to other directories. However, this series is not splitting > files, only moving them. The purpose is to get out of the local optimum > we're stuck in). I could live with a rule that says "no devices in hw/$TARGET unless they're there for legacy reasons because they share a source file with a machine model" [and ideally eventually pull any self-contained devices out of those files]. That's a nice clear line for reviewing new patches. Why, for example, is arm_gic.c moved to hw/arm but hw/heathrow_pic.c moved to hw/misc rather than hw/ppc ? -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Support setgroups syscall with no groups
On 4 February 2013 18:38, Eric Blake wrote:
> On 02/02/2013 04:04 PM, dill...@dillona.com wrote:
>> -
>> -grouplist = alloca(gidsetsize * sizeof(gid_t));
>> -target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize * 2,
>> 1);
>> -if (!target_grouplist) {
>> -ret = -TARGET_EFAULT;
>> -goto fail;
>> +if (gidsetsize) {
>> +grouplist = alloca(gidsetsize * sizeof(gid_t));
>
> Is this alloca() safe, or are you risking stack overflow if the user
> passes an extremely large arg1?
No, the linux-user has a number of long-standing not-terribly-safe
alloca calls like this. If anybody wants to go through and fix them
patches are welcome, but I don't think it's fair to require them
to be fixed in order to get fairly simple patches like this in,
where the patch is merely reindenting existing dubious code, not
adding to the problem.
-- PMM
Re: [Qemu-devel] [PATCH][v5] linux-user: correct semctl() and shmctl()
Le lundi 04 février 2013 à 15:16 +, Peter Maydell a écrit :
> On 31 January 2013 19:50, Laurent Vivier wrote:
> > The parameter "union semun" of semctl() is not a value
> > but a pointer to the value.
>
> Hi. For your next patch could you make sure you send it as
> a fresh email rather than a followup to the previous version?
> Anthony's patch-handling tools don't really like followups.
OK
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -2652,8 +2652,9 @@ static inline abi_long host_to_target_semarray(int
> > semid, abi_ulong target_addr,
> > }
> >
> > static inline abi_long do_semctl(int semid, int semnum, int cmd,
> > - union target_semun target_su)
> > + abi_ulong ptr)
> > {
> > +union target_semun *target_su;
> > union semun arg;
> > struct semid_ds dsarg;
> > unsigned short *array = NULL;
> > @@ -2662,43 +2663,55 @@ static inline abi_long do_semctl(int semid, int
> > semnum, int cmd,
> > abi_long err;
> > cmd &= 0xff;
> >
> > +if (!lock_user_struct(VERIFY_READ, target_su, ptr, 1)) {
> > +return -TARGET_EFAULT;
> > +}
>
> This breaks x86_64 linux-user. The fourth argument to semctl()
> is a union of pointers, not a pointer to a union. That means that
In fact, it depends on the architecture. After a look in the kernel
sources, it seems compat_sys_semctl() uses a pointer, sys_semctl() an
union. compat_sys_semctl() seems to be used by mips32, pp32, sparc32 and
x86_32.
> the lock_user_struct/whatever has to be done differently for the
> individual cases, depending on how we are supposed to interpret
> the argument (which field of the union we're using).
>
> My testcase is simple:
>
> QEMU_STRACE=1 ./x86_64-linux-user/qemu-x86_64 /usr/bin/ipcs
>
> which before your patch does this:
>
> 14654 semctl(0,0,SEM_INFO,0x004000800490) = 0
> 14654 write(1,0x10d4000,33)-- Semaphore Arrays
> = 33
>
> (ie we successfully get back the info)
>
> 14654 write(1,0x10d4000,55)keysemid owner perms
> nsems
> = 55
> 14654 semctl(0,0,SEM_STAT,0x004000800420) = -1 errno=22 (Invalid argument)
> 14654 write(1,0x10d4000,1)
> = 1
>
> and afterwards does this:
>
> 14723 semctl(0,0,SEM_INFO,0x004000800490) = -1 errno=14 (Bad address)
> 14723 write(1,0x10d4000,37)kernel not configured for semaphores
> = 37
>
> (SEM_INFO fails and ipcs prints a failure message)
>
> because we end up with target_su->__buf == 11 which isn't a
> valid address to pass to host_to_target_seminfo().
Thank you for your help,
Laurent
--
"Just play. Have fun. Enjoy the game."
- Michael Jordan
Re: [Qemu-devel] [PATCH 00/19] hw/ directory restructuring
Il 04/02/2013 21:33, Peter Maydell ha scritto: > On 4 February 2013 18:59, Paolo Bonzini wrote: >> Il 04/02/2013 19:13, Peter Maydell ha scritto: >>> I think this is inconsistent. Either we should organise by category, >>> or we should organise per-target, but we shouldn't try to do both. >>> Otherwise we're just going to wind up with half the $TARGET-specific >>> $FOO device models in hw/$TARGET and the other half in hw/$FOO. >> >> The $FOO device models should all be in hw/$FOO. But there are some >> parts that do not fit in a category > > Surely this is what hw/misc is for? It's mostly interrupt/GPIO/DMA/memory/cache controllers. I placed some of them are in hw/misc because they are in common-obj-y, but I don't like the idea of hw/misc very much. hw/misc is really for the one-offs such as the tmp105.c temperature controller. If you prefer with creating a bunch of hw/ subdirectories like hw/pic, hw/gpio, hw/dma I can certainly move more stuff out of hw/$TARGET. As long as there is consensus, it's fine. But I wanted to avoid proliferation of hw/$FOO directories having only 5-10 files. The final submission may not have an hw/misc at all, the odd device like tmp105.c can go in hw/i2c. > (Also a hw/machines might be a good idea.) I don't like it very much because it's not clear which targets are used for which machines. I want to move away from the hw/arm/../foo.o hack that is used now. >> or are always target-specific, and >> hw/$TARGET has them. >> >> Also, the kernel similarly has arch/$TARGET and drivers/$FOO. > > Yes, and the ARM kernel maintainers have just spent quite a lot > of effort in cleaning things up to kick all the drivers out > of arch/$TARGET and into drivers/$FOO where they belong. > I'd rather we didn't get into that mess in the first place :-) Yes, and in fact I'm moving a lot of ARM devices away from hw/arm's Makefile.objs to other directories (net/ for NICs, char/ for UARTs etc.), and configuring them via default-configs/arm-softmmu.mak. But Linux still has a bunch of board-specific drivers in arch/arm/mach-* that won't be moved to drivers/, and that's roughly what I left in hw/$TARGET. (Note: some boards still have devices in hw/arm after these patches, e.g. musicpal has a NIC and LCD controller. Those could indeed be refactored to other directories. However, this series is not splitting files, only moving them. The purpose is to get out of the local optimum we're stuck in). Paolo
Re: [Qemu-devel] [PATCH for-1.4] libi2c-omap: Fix endianness dependency
Peter Maydell writes: > On 2 February 2013 18:26, Blue Swirl wrote: >> On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell >> wrote: >>> On 2 February 2013 17:37, Andreas Färber wrote: Am 02.02.2013 17:49, schrieb Peter Maydell: libqtest.h has no generic endian-aware memread functions unlike Alex, you or me expected. It reads a sequence of bytes from guest memory and transmits them one-by-one over the text-based qtest protocol. >>> >>> OK, so this is just busted for accessing devices. The protocol >>> has to have some way of letting you do a 32 bit / 16 bit / 8 bit >>> access (and maybe 64 bit as well while we're here). memread >>> and memwrite are OK for RAM accesses [ie anything you'd be >>> happy to have cached or buffered in a real system] but for >>> memory mapped registers we need to have an equivalent of >>> inb/inw/inl/outb/outw/outl that guarantee to do exactly one >>> access of exactly the required width. >> >> I was also just making a patch (but not so nice as Andreas'). My >> analysis was that qtest.c and libqtest.c just pass the result of >> cpu_physical_memory_rw() as is, with no endian conversion. So the >> result needs to be converted to host CPU order just like Andreas did. >> I think the protocol is OK, my initial reaction was to put byte >> swapping there but that's not right. > > No, I think the protocol is broken, even if you ignore the > question of endianness. Consider that a device's MemoryRegion > can have different behaviour depending on whether you access it > as a byte, halfword or word size. cpu_physical_memory_rw() won't > let you make a word size access to an unaligned address. memread/memwrite is for bulk data transfer. It's a byte API. It would be a good idea to add word-sized I/O requests to the protocol. Regards, Anthony Liguori > > -- PMM
Re: [Qemu-devel] [PATCH 00/19] hw/ directory restructuring
On 4 February 2013 18:59, Paolo Bonzini wrote: > Il 04/02/2013 19:13, Peter Maydell ha scritto: >> I think this is inconsistent. Either we should organise by category, >> or we should organise per-target, but we shouldn't try to do both. >> Otherwise we're just going to wind up with half the $TARGET-specific >> $FOO device models in hw/$TARGET and the other half in hw/$FOO. > > The $FOO device models should all be in hw/$FOO. But there are some > parts that do not fit in a category Surely this is what hw/misc is for? (Also a hw/machines might be a good idea.) > or are always target-specific, and > hw/$TARGET has them. > > Also, the kernel similarly has arch/$TARGET and drivers/$FOO. Yes, and the ARM kernel maintainers have just spent quite a lot of effort in cleaning things up to kick all the drivers out of arch/$TARGET and into drivers/$FOO where they belong. I'd rather we didn't get into that mess in the first place :-) -- PMM
Re: [Qemu-devel] [PATCH WIP 0/4] vhost-scsi: new device supporting the tcm_vhost Linux kernel module
Hi Paolo,
On Thu, 2013-01-31 at 00:12 -0800, Nicholas A. Bellinger wrote:
> Hi Paolo,
>
> On Wed, 2013-01-30 at 17:41 +0100, Paolo Bonzini wrote:
> > As expected, using a separate device finds a snag: vhost-scsi is passing
> > force=false to vhost_dev_init, and the BIOS does not use MSI-X so it
> > will actually use the non-vhost implementation which is wrong. I fixed
> > this by passing force=true; I'm not sure what that would break, but I
> > figured "not much" since the BIOS polls and does not rely on interrupts.
> >
> > That makes vhost start, but it still doesn't work for me with a 3.7.2
> > kernel on the host. Even Nick's patches hang the guest as soon as vhost
> > starts, and I get the same behavior with mine.
>
> After bisection this evening, the change that ended up breaking
> vhost-scsi is vhost backend guest notifier masking support patch in
> commit f56a12475.
>
> After adding the two new notifiers in vhost-scsi/virtio-scsi following
> in qemu.git/virtio-scsi-nab code, the tcm_vhost LUN scan is functioning
> again..
>
> > (Of course with my
> > patches the BIOS hangs and you never reach Linux; but try a BIOS without
> > virtio-scsi support, and you'll see Linux hanging in the same way).
> >
> > Here is my configuration:
> >
> > cd /sys/kernel/config/target
> > mkdir -p core/fileio_0/fileio
> > echo 'fd_dev_name=/home/pbonzini/test.img,fd_dev_size=5905580032' >
> > core/fileio_0/fileio/control
> > echo 1 > core/fileio_0/fileio/enable
> > mkdir -p vhost/naa.600140554cf3a18e/tpgt_0/lun/lun_0
> > cd vhost/naa.600140554cf3a18e/tpgt_0
> > ln -sf ../../../../../core/fileio_0/fileio/ lun/lun_0/virtual_scsi_port
> > echo naa.60014053226f0388 > nexus
> >
> > Nick's patches are run with "-vhost-scsi
> > id=vs,tpgt=0,wwpn=naa.600140554cf3a18e
> > -device virtio-scsi-pci,vhost-scsi=vs". Perhaps I'm doing something wrong.
>
> So after adding the same vhost backend guest notifiers to the new
> VirtIOSCSICommon vhost-scsi code, I'm now hitting an QEMU invalid
> option:
>
> ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -smp 4 -m 2048 -serial
> file:/tmp/vhost-serial.txt
> -hda /usr/src/qemu-vhost.git/debian_squeeze_amd64_standard-old.qcow2
> -vhost-scsi id=vs,tpgt=0,wwpn=naa.600140579ad21088 -device
> virtio-scsi-pci,vhost-scsi=vs
> qemu-system-x86_64: -vhost-scsi: invalid option
>
> Debugging this now..
>
Ok, so after using the correct -vhost-scsi-pci opts with thew new code
(thanks btw :), I'm also running into a SeaBIOS hang at boot:
Starting program: /usr/src/qemu-paolo.git/x86_64-softmmu/qemu-system-x86_64
-enable-kvm -smp 4 -m 2048 -serial file:/tmp/vhost-serial.txt -hda
/usr/src/qemu-vhost.git/debian_squeeze_amd64_standard-old.qcow2 -device
vhost-scsi-pci,wwpn=naa.600140579ad21088
[Thread debugging using libthread_db enabled]
[New Thread 0x743a1700 (LWP 13079)]
[New Thread 0x7399f700 (LWP 13080)]
[New Thread 0x7319e700 (LWP 13081)]
[New Thread 0x7299d700 (LWP 13082)]
[New Thread 0x7219c700 (LWP 13083)]
[Thread 0x743a1700 (LWP 13079) exited]
^C
Program received signal SIGINT, Interrupt.
0x75cca8d3 in select () from /lib/libc.so.6
(gdb) bt
#0 0x75cca8d3 in select () from /lib/libc.so.6
#1 0x005050e5 in os_host_main_loop_wait (nonblocking=) at main-loop.c:230
#2 main_loop_wait (nonblocking=) at main-loop.c:416
#3 0x0056a8ec in main_loop (argc=, argv=,
envp=) at vl.c:1951
#4 main (argc=, argv=, envp=) at vl.c:4224
(gdb)
and a quick look with strace:
ioctl(15, KVM_SET_LAPIC, 0x7fffca93f6e0) = 0
ioctl(15, KVM_SET_PIT2 or KVM_SET_VCPU_EVENTS, 0x7fffca93f6e0) = 0
ioctl(15, 0x4080aea2, 0x7fffca93f6e0) = 0
write(4, "\1\0\0\0\0\0\0\0", 8) = 8
write(4, "\1\0\0\0\0\0\0\0", 8) = 8
ioctl(7, KVM_CHECK_EXTENSION, 0x4c) = 1
ioctl(12, 0xaead, 0)= -1 EINVAL (Invalid argument)
futex(0x1469ce4, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 0xe54e20, 4) = 1
tgkill(13107, 13109, SIGUSR1) = 0
futex(0x1460114, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 0xe54e20, 4) = 1
tgkill(13107, 13110, SIGUSR1) = 0
futex(0x14613b4, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 0xe54e20, 4) = 1
tgkill(13107, 13111, SIGUSR1) = 0
futex(0x1454e84, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 0xe54e20, 4) = 1
tgkill(13107, 13112, SIGUSR1) = 0
select(10, [3 4 5 9], [], [], {0, 0}) = 2 (in [3 4], left {0, 0})
read(4, "\2\0\0\0\0\0\0\0", 512)= 8
read(3,
"\16\0\0\0\0\0\0\0\376\377\377\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
128) = 128
rt_sigaction(SIGALRM, NULL, {0x52b810, ~[KILL STOP RTMIN RT_1], SA_RESTORER,
0x7f2c559abf60}, 8) = 0
write(4, "\1\0\0\0\0\0\0\0", 8) = 8
read(3, 0x7fffca93fc60, 128)= -1 EAGAIN (Resource temporarily
unavailable)
poll([{fd=20, events=POLLIN|POLLOUT}], 1, -1) = 1 ([{fd=20, revents=POLLOUT}])
writev(20,
[{"\22\0\7\0\3\0\0\6'\0\0\0\37\0\0\0\10\1\4\0\4\0\0\0QEMU\22\0\7\0"..., 116},
{NULL, 0}, {"
Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses
On Mon, 4 Feb 2013 17:05:01 +0100
Igor Mammedov wrote:
> On Mon, 04 Feb 2013 13:52:32 +0100
> Andreas Färber wrote:
>
> > Am 04.02.2013 12:08, schrieb Igor Mammedov:
> > > On Sat, 2 Feb 2013 01:37:07 +0100
> > > Andreas Färber wrote:
> > >
[...]
>
> > >> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
> > >> CPUState *cs = CPU(obj);
> > >> X86CPU *cpu = X86_CPU(obj);
> > >> CPUX86State *env = &cpu->env;
> > >> +X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> > >> +const x86_def_t *def = &xcc->info;
> > >> static int inited;
> > >>
> > >> cpu_exec_init(env);
> > >> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
> > >> x86_cpuid_get_tsc_freq,
> > >> x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> > >>
> > >> +/* sysenter isn't supported in compatibility mode on AMD,
> > >> + * syscall isn't supported in compatibility mode on Intel.
> > >> + * Normally we advertise the actual CPU vendor, but you can
> > >> + * override this using the 'vendor' property if you want to use
> > >> + * KVM's sysenter/syscall emulation in compatibility mode and
> > >> + * when doing cross vendor migration
> > >> + */
> > >> +if (kvm_enabled()) {
> > >> +host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
> > >> + &env->cpuid_vendor3);
> > > This is not per instance specific, this override would be better done to
> > > sub-classes in kvm_arch_init().
> >
> > Hmm... I think the issue I addresses this way was that kvm.c didn't have
> > access to your static vendor_words2str() helper. Writing the words into
> > the word fields is more direct than converting them to a string and
> > writing them back into the words.
> you have these correct vendor words ready after you called
> kvm_host_cpu_class_init() in kvm_arch_init(), so just direct copy from host
> class would do.
I was talking nonsense here.
Why not to make vendor_words2str() helper not static and use it in kvm.c
instead of doing partial init of host class and then reuse the value you got
to override vendor in other classes?
[...]
--
Regards,
Igor
Re: [Qemu-devel] [PATCH v2] MIPS: Translate breaks and traps into the appropriate signal
Ping: http://patchwork.ozlabs.org/patch/211162/.
On 01/10/2013 04:50 PM, Meador Inge wrote:
> GCC and GAS are capable of generating traps or breaks to check for
> division by zero. Additionally, GAS is capable of generating traps
> or breaks to check for overflow on certain division and multiplication
> operations. The Linux kernel translates these traps and breaks into
> signals. This patch implements the corresponding feature in QEMU.
>
> Signed-off-by: Meador Inge
> ---
> Changes since v1:
>
> * Moved the BRK_* enumerations from target-mips/cpu.h to
> linux-user/main.c since they are only used in main.c
>
> * Fixed some style violations found by checkpatch.pl.
>
> * Removed a superfluous break.
>
> linux-user/main.c | 76
> -
> 1 file changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 9ade1bf..583940c 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -2182,6 +2182,33 @@ static int do_store_exclusive(CPUMIPSState *env)
> return segv;
> }
>
> +/* Break codes */
> +enum {
> +BRK_OVERFLOW = 6,
> +BRK_DIVZERO = 7
> +};
> +
> +static int do_break(CPUMIPSState *env, target_siginfo_t *info,
> +unsigned int code)
> +{
> +int ret = -1;
> +
> +switch (code) {
> +case BRK_OVERFLOW:
> +case BRK_DIVZERO:
> +info->si_signo = TARGET_SIGFPE;
> +info->si_errno = 0;
> +info->si_code = (code == BRK_OVERFLOW) ? FPE_INTOVF : FPE_INTDIV;
> +queue_signal(env, info->si_signo, &*info);
> +ret = 0;
> +break;
> +default:
> +break;
> +}
> +
> +return ret;
> +}
> +
> void cpu_loop(CPUMIPSState *env)
> {
> target_siginfo_t info;
> @@ -2297,8 +2324,55 @@ done_syscall:
> info.si_code = TARGET_ILL_ILLOPC;
> queue_signal(env, info.si_signo, &info);
> break;
> +/* The code below was inspired by the MIPS Linux kernel trap
> + * handling code in arch/mips/kernel/traps.c.
> + */
> +case EXCP_BREAK:
> +{
> +abi_ulong trap_instr;
> +unsigned int code;
> +
> +ret = get_user_ual(trap_instr, env->active_tc.PC);
> +if (ret != 0) {
> +goto error;
> +}
> +
> +/* As described in the original Linux kernel code, the
> + * below checks on 'code' are to work around an old
> + * assembly bug.
> + */
> +code = ((trap_instr >> 6) & ((1 << 20) - 1));
> +if (code >= (1 << 10)) {
> +code >>= 10;
> +}
> +
> +if (do_break(env, &info, code) != 0) {
> +goto error;
> +}
> +}
> +break;
> +case EXCP_TRAP:
> +{
> +abi_ulong trap_instr;
> +unsigned int code = 0;
> +
> +ret = get_user_ual(trap_instr, env->active_tc.PC);
> +if (ret != 0) {
> +goto error;
> +}
> +
> +/* The immediate versions don't provide a code. */
> +if (!(trap_instr & 0xFC00)) {
> +code = ((trap_instr >> 6) & ((1 << 10) - 1));
> +}
> +
> +if (do_break(env, &info, code) != 0) {
> +goto error;
> +}
> +}
> +break;
> default:
> -//error:
> +error:
> fprintf(stderr, "qemu: unhandled CPU exception 0x%x -
> aborting\n",
> trapnr);
> cpu_dump_state(env, stderr, fprintf, 0);
>
--
Meador Inge
CodeSourcery / Mentor Embedded
[Qemu-devel] [PATCH v3 00/10] main-loop: switch to g_poll(3) on POSIX hosts
Amos Kong reported that file descriptors numbered higher than 1024 could crash QEMU. This is due to the fixed size of the fd_set type used for select(2) event polling. This series converts the main-loop.c and aio-posix.c select(2) calls to g_poll(3). This eliminates the fd_set type and allows QEMU to scale to high numbers of file descriptors. The g_poll(3) interface is a portable version of the poll(2) system call. The difference to select(2) is that fine-grained events (G_IO_IN, G_IO_OUT, G_IO_HUP, G_IO_ERR, G_IO_PRI) can be monitored instead of just read/write/exception. Also, there is no limit to the file descriptor numbers that may be used, allowing applications to scale to many file descriptors. See the documentation for details: http://developer.gnome.org/glib/2.28/glib-The-Main-Event-Loop.html#g-poll The QEMU main loop works as follows today: 1. Call out to slirp, iohandlers, and glib sources to fill rfds/wfds/xfds with the file descriptors to select(2). 2. Perform the select(2) call. 3. Call out to slirp, iohandlers, and glib sources to handle events polled in rfds/wfds/xfds. The plan of attack is as follows: 1. Replace select(2) with g_poll(3). Use glue that converts between rfds/wfds/xfds and GPollFD so that the unconverted QEMU components still work. 2. Convert slirp, iohandlers, and glib source fill/poll functions to use GPollFD directly instead of rfds/wfds/xfds. 3. Drop the glue since all components now natively use GPollFD. 4. Convert aio-posix.c to g_poll(3) by reusing GPollFD. I have tested that the series builds and is bisectable on Linux and Windows hosts. But I have not done extensive testing on other host platforms or with long-term guests to check for performance regressions. v3: * Convert slirp/slirp.c to tabs to spaces [Blue Swirl] v2: * Replace custom Poller type with GArray [aliguori] Stefan Hajnoczi (10): main-loop: fix select_ret uninitialized variable warning main-loop: switch to g_poll() on POSIX hosts main-loop: switch POSIX glib integration to GPollFD slirp: slirp/slirp.c coding style cleanup slirp: switch to GPollFD iohandler: switch to GPollFD main-loop: drop rfds/wfds/xfds for good aio: extract aio_dispatch() from aio_poll() aio: convert aio_poll() to g_poll(3) aio: support G_IO_HUP and G_IO_ERR aio-posix.c | 130 - async.c | 2 + include/block/aio.h | 3 + include/qemu/main-loop.h | 4 +- iohandler.c | 40 ++- main-loop.c | 156 ++- slirp/libslirp.h | 6 +- slirp/main.h | 1 - slirp/slirp.c| 673 +-- slirp/socket.c | 9 - slirp/socket.h | 2 + stubs/slirp.c| 6 +- 12 files changed, 545 insertions(+), 487 deletions(-) -- 1.8.1
[Qemu-devel] [PATCH 60/60] migration: Fix madvise breakage if host and guest have different page sizes
From: David Gibson
madvise(DONTNEED) will throw away the contents of the whole page at the
given address, even if the given length is less than the page size. One
can argue about whether that's the correct behaviour, but that's what it's
done for a long time in Linux at least.
That means that the madvise() in ram_load(), on a setup where
TARGET_PAGE_SIZE is smaller than the host page size, can throw away data
in guest pages adjacent to the one it's actually processing right now,
leading to guest memory corruption on an incoming migration.
This patch therefore, disables the madvise() if the host page size is
larger than TARGET_PAGE_SIZE. This means we don't get the benefits of that
madvise() in this case, but a more complete fix is more difficult to
accomplish. This at least fixes the guest memory corruption.
Signed-off-by: David Gibson
Reported-by: Alexey Kardashevskiy
Signed-off-by: Anthony Liguori
(cherry picked from commit 45e6cee42b98d10e2e14885ab656541a9ffd5187)
Signed-off-by: Michael Tokarev
---
arch_init.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch_init.c b/arch_init.c
index a9e8b74..a1c3cfb 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -475,7 +475,8 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
memset(host, ch, TARGET_PAGE_SIZE);
#ifndef _WIN32
if (ch == 0 &&
-(!kvm_enabled() || kvm_has_sync_mmu())) {
+(!kvm_enabled() || kvm_has_sync_mmu()) &&
+getpagesize() <= TARGET_PAGE_SIZE) {
qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
}
#endif
--
1.7.10.4
[Qemu-devel] [PATCH 04/60] configure: Fix CONFIG_QEMU_HELPERDIR generation
From: Jan Kiszka
We need to evaluate $libexecdir in configure, otherwise we literally end
up with "${prefix}/libexec" instead of the absolute path as
CONFIG_QEMU_HELPERDIR.
Signed-off-by: Jan Kiszka
Signed-off-by: Aurelien Jarno
(cherry picked from commit 38f419f35225decdbaea9fe1fd00218f8924ce84)
---
configure |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 1769e5f..d5c0a47 100755
--- a/configure
+++ b/configure
@@ -3072,7 +3072,7 @@ echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
-echo "CONFIG_QEMU_HELPERDIR=\"$libexecdir\"" >> $config_host_mak
+echo "CONFIG_QEMU_HELPERDIR=\"`eval echo $libexecdir`\"" >> $config_host_mak
echo "ARCH=$ARCH" >> $config_host_mak
if test "$debug_tcg" = "yes" ; then
--
1.7.10.4
Re: [Qemu-devel] [PATCH 6/7] qbus_find(): report errors via Error
On Mon, 04 Feb 2013 19:23:15 +0100 Laszlo Ersek wrote: > On 02/04/13 18:51, Luiz Capitulino wrote: > > On Fri, 1 Feb 2013 18:38:18 +0100 > > Laszlo Ersek wrote: > > > >> > >> Signed-off-by: Laszlo Ersek > > > > Splitting this as I suggested in the other patch would make review > > easier. I honestly got a bit lost while reviewing this one. > > I think you cannot review this series without applying each patch in > lock-step with the review, and contrasting the next patch with the > just-updated tree. In that light it's especially regrettable that my > series didn't apply to master (it certainly did when I posted it). I'll review it in more detail when you respin. > > Anyway the approach I took was: > - determine the set of functions to modify, > - work in a bottom-up fashion, > - when changing a function signature, change all callers. > > Suppose there are three functions, X, Y, Z, with calls like X->Y, X->Z. > > #1 First I'd fix Y (signature change) and adapt its call site in X -- > acccept the error and print it. At this point X consumes a few errors, > and some other errors don't reach it at all. May work for simple cases (although has the minor problem of mixing signature change with adding new error handling to X), but doesn't scale when Y is called by more than one function and their error handling differ. > > #2 Then I'd fix Z (signature change) and adapt its call site in X -- > accept the error and print it. At this point Y and Z are OK, and X has > several places that accept and consume (print/free) errors. > > #3 In the next patch, X's signature is changed, all error consumption / > printing sites in X are changed to propagate / generate instead, and all > call sites or X are updated. > > The only patch I could split is #3, but then X would in part consume, in > part propagate errors. Do you suggest I do that? The suggestion I gave in another email is to separate signature changes from error handling changes. Taking your example above, I'd have the following patches: 1. Change Y's signature to take an Error ** argument and change X to pass NULL for the new argument 2. Change Z's signature to take an Error ** argument and change X to pass NULL for the new argument 3. Change X to pass an non-NULL Error ** argument to Y and Z and handle error accordingly 4. If Y is non-void and if the return value is success/failure, drop it 5. If Y is non-void and if the return value is success/failure, drop it Now, you could merge patches 1 and 2 if the functions are only used by X. On the other hand, if Y and/or Z are called by other functions, you'd have to add more patches after 3 to add error handling to them. I know this is more work, but it makes it easier to review.
[Qemu-devel] [PATCH 25/60] hw: Fix return value check for bdrv_read, bdrv_write
From: Stefan Weil
Those functions return -errno in case of an error.
The old code would typically only detect EPERM (1) errors.
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
(cherry picked from commit 7a608f562ebd91e811ed0b725e528c894e4f19c4)
Signed-off-by: Michael Tokarev
---
hw/nand.c| 34 ++
hw/onenand.c |2 +-
hw/sd.c | 16 +---
3 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/hw/nand.c b/hw/nand.c
index e9501ae..01f3ada 100644
--- a/hw/nand.c
+++ b/hw/nand.c
@@ -654,7 +654,7 @@ static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState
*s)
sector = SECTOR(s->addr);
off = (s->addr & PAGE_MASK) + s->offset;
soff = SECTOR_OFFSET(s->addr);
-if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS) == -1) {
+if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS) < 0) {
printf("%s: read error in sector %" PRIu64 "\n", __func__, sector);
return;
}
@@ -666,21 +666,23 @@ static void glue(nand_blk_write_,
PAGE_SIZE)(NANDFlashState *s)
MIN(OOB_SIZE, off + s->iolen - PAGE_SIZE));
}
-if (bdrv_write(s->bdrv, sector, iobuf, PAGE_SECTORS) == -1)
+if (bdrv_write(s->bdrv, sector, iobuf, PAGE_SECTORS) < 0) {
printf("%s: write error in sector %" PRIu64 "\n", __func__,
sector);
+}
} else {
off = PAGE_START(s->addr) + (s->addr & PAGE_MASK) + s->offset;
sector = off >> 9;
soff = off & 0x1ff;
-if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS + 2) == -1) {
+if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS + 2) < 0) {
printf("%s: read error in sector %" PRIu64 "\n", __func__, sector);
return;
}
mem_and(iobuf + soff, s->io, s->iolen);
-if (bdrv_write(s->bdrv, sector, iobuf, PAGE_SECTORS + 2) == -1)
+if (bdrv_write(s->bdrv, sector, iobuf, PAGE_SECTORS + 2) < 0) {
printf("%s: write error in sector %" PRIu64 "\n", __func__,
sector);
+}
}
s->offset = 0;
}
@@ -704,31 +706,37 @@ static void glue(nand_blk_erase_,
PAGE_SIZE)(NANDFlashState *s)
i = SECTOR(addr);
page = SECTOR(addr + (ADDR_SHIFT + s->erase_shift));
for (; i < page; i ++)
-if (bdrv_write(s->bdrv, i, iobuf, 1) == -1)
+if (bdrv_write(s->bdrv, i, iobuf, 1) < 0) {
printf("%s: write error in sector %" PRIu64 "\n", __func__, i);
+}
} else {
addr = PAGE_START(addr);
page = addr >> 9;
-if (bdrv_read(s->bdrv, page, iobuf, 1) == -1)
+if (bdrv_read(s->bdrv, page, iobuf, 1) < 0) {
printf("%s: read error in sector %" PRIu64 "\n", __func__, page);
+}
memset(iobuf + (addr & 0x1ff), 0xff, (~addr & 0x1ff) + 1);
-if (bdrv_write(s->bdrv, page, iobuf, 1) == -1)
+if (bdrv_write(s->bdrv, page, iobuf, 1) < 0) {
printf("%s: write error in sector %" PRIu64 "\n", __func__, page);
+}
memset(iobuf, 0xff, 0x200);
i = (addr & ~0x1ff) + 0x200;
for (addr += ((PAGE_SIZE + OOB_SIZE) << s->erase_shift) - 0x200;
i < addr; i += 0x200)
-if (bdrv_write(s->bdrv, i >> 9, iobuf, 1) == -1)
+if (bdrv_write(s->bdrv, i >> 9, iobuf, 1) < 0) {
printf("%s: write error in sector %" PRIu64 "\n",
__func__, i >> 9);
+}
page = i >> 9;
-if (bdrv_read(s->bdrv, page, iobuf, 1) == -1)
+if (bdrv_read(s->bdrv, page, iobuf, 1) < 0) {
printf("%s: read error in sector %" PRIu64 "\n", __func__, page);
+}
memset(iobuf, 0xff, ((addr - 1) & 0x1ff) + 1);
-if (bdrv_write(s->bdrv, page, iobuf, 1) == -1)
+if (bdrv_write(s->bdrv, page, iobuf, 1) < 0) {
printf("%s: write error in sector %" PRIu64 "\n", __func__, page);
+}
}
}
@@ -740,18 +748,20 @@ static void glue(nand_blk_load_,
PAGE_SIZE)(NANDFlashState *s,
if (s->bdrv) {
if (s->mem_oob) {
-if (bdrv_read(s->bdrv, SECTOR(addr), s->io, PAGE_SECTORS) == -1)
+if (bdrv_read(s->bdrv, SECTOR(addr), s->io, PAGE_SECTORS) < 0) {
printf("%s: read error in sector %" PRIu64 "\n",
__func__, SECTOR(addr));
+}
memcpy(s->io + SECTOR_OFFSET(s->addr) + PAGE_SIZE,
s->storage + (PAGE(s->addr) << OOB_SHIFT),
OOB_SIZE);
s->ioaddr = s->io + SECTOR_OFFSET(s->addr) + offset;
} else {
if (bdrv_read(s->bdrv, PAGE_START(addr) >> 9,
-s->io, (PAGE_SECTORS + 2)) == -1)
+s->io, (PAGE_SECTORS + 2)) < 0) {
printf("%s: read error in s
[Qemu-devel] [PATCH v3 01/10] main-loop: fix select_ret uninitialized variable warning
Signed-off-by: Stefan Hajnoczi
---
main-loop.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/main-loop.c b/main-loop.c
index 6f52ac3..d0d8fe4 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -330,7 +330,8 @@ void qemu_fd_register(int fd)
static int os_host_main_loop_wait(uint32_t timeout)
{
GMainContext *context = g_main_context_default();
-int select_ret, g_poll_ret, ret, i;
+int select_ret = 0;
+int g_poll_ret, ret, i;
PollingEntry *pe;
WaitObjects *w = &wait_objects;
gint poll_timeout;
--
1.8.1
[Qemu-devel] [PATCH 03/19] hw: move qdev-monitor.o to toplevel directory
qdev-monitor.c is the only "core qdev" file that is not used in user-mode emulation, and it does not define anything that is used by hardware models. Remove it from the hw/ directory and remove hw/qdev-monitor.h from hw/qdev.h too; this requires some files to have some new explicitly includes. Signed-off-by: Paolo Bonzini --- Makefile.objs |1 + hw/9pfs/virtio-9p-proxy.c |1 + hw/Makefile.objs|1 - hw/pc87312.c|1 + hw/pc_sysfw.c |1 + hw/pci/shpc.c |3 ++- hw/pci/slotid_cap.c |1 + hw/qdev-addr.c |1 + hw/qdev.c |1 + hw/qdev.h |1 - hw/s390x/sclpconsole.c |1 + hw/usb/dev-network.c|1 + hw/virtio-rng.c |1 + hw/virtio-scsi.c|1 + hw/xilinx.h |3 ++- hw/qdev-monitor.h => include/monitor/qdev.h |3 +-- monitor.c |2 +- hw/qdev-monitor.c => qdev-monitor.c |3 ++- util/qemu-config.c |1 + vl.c|1 + 20 files changed, 21 insertions(+), 8 deletions(-) rename hw/qdev-monitor.h => include/monitor/qdev.h (80%) rename hw/qdev-monitor.c => qdev-monitor.c (99%) diff --git a/Makefile.objs b/Makefile.objs index 21e9c91..f37e701 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -51,6 +51,7 @@ ifeq ($(CONFIG_SOFTMMU),y) common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/ common-obj-y += net/ common-obj-y += readline.o +common-obj-y += qdev-monitor.o common-obj-$(CONFIG_WIN32) += os-win32.o common-obj-$(CONFIG_POSIX) += os-posix.o diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c index d5ad208..acac15c 100644 --- a/hw/9pfs/virtio-9p-proxy.c +++ b/hw/9pfs/virtio-9p-proxy.c @@ -13,6 +13,7 @@ #include #include "hw/virtio.h" #include "virtio-9p.h" +#include "qemu/error-report.h" #include "fsdev/qemu-fsdev.h" #include "virtio-9p-proxy.h" diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 6aad5de..ca7d5bd 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -187,7 +187,6 @@ common-obj-$(CONFIG_SD) += sd.o common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o common-obj-y += bt-hci-csr.o common-obj-y += ps2.o -common-obj-y += qdev-monitor.o common-obj-y += qdev-properties-system.o # xen backend driver support diff --git a/hw/pc87312.c b/hw/pc87312.c index 38af4c1..0e9760e 100644 --- a/hw/pc87312.c +++ b/hw/pc87312.c @@ -24,6 +24,7 @@ */ #include "pc87312.h" +#include "qemu/error-report.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "char/char.h" diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index 7f6c12c..8b65a7a 100644 --- a/hw/pc_sysfw.c +++ b/hw/pc_sysfw.c @@ -24,6 +24,7 @@ */ #include "sysemu/blockdev.h" +#include "qemu/error-report.h" #include "sysbus.h" #include "hw.h" #include "pc.h" diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c index f07266d..d35c2ee 100644 --- a/hw/pci/shpc.c +++ b/hw/pci/shpc.c @@ -1,7 +1,8 @@ +#include "qemu-common.h" #include #include #include "qemu/range.h" -#include "qemu/range.h" +#include "qemu/error-report.h" #include "hw/pci/shpc.h" #include "hw/pci/pci.h" #include "hw/pci/pci_bus.h" diff --git a/hw/pci/slotid_cap.c b/hw/pci/slotid_cap.c index 99a30f4..62f7bae 100644 --- a/hw/pci/slotid_cap.c +++ b/hw/pci/slotid_cap.c @@ -1,5 +1,6 @@ #include "hw/pci/slotid_cap.h" #include "hw/pci/pci.h" +#include "qemu/error-report.h" #define SLOTID_CAP_LENGTH 4 #define SLOTID_NSLOTS_SHIFT (ffs(PCI_SID_ESR_NSLOTS) - 1) diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c index b4388f6..fc2c437 100644 --- a/hw/qdev-addr.c +++ b/hw/qdev-addr.c @@ -1,6 +1,7 @@ #include "qdev.h" #include "qdev-addr.h" #include "exec/hwaddr.h" +#include "qapi/qmp/qerror.h" #include "qapi/visitor.h" /* --- target physical address --- */ diff --git a/hw/qdev.c b/hw/qdev.c index 8258757..cd18026 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -28,6 +28,7 @@ #include "qdev.h" #include "sysemu/sysemu.h" #include "qapi/error.h" +#include "qapi/qmp/qerror.h" #include "qapi/visitor.h" int qdev_hotplug = 0; diff --git a/hw/qdev.h b/hw/qdev.h index 365b8d6..f814656 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -4,6 +4,5 @@ #include "hw/hw.h" #include "qdev-core.h" #include "qdev-properties.h" -#include "qdev-monitor.h" #endif diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c index adc0ee8..c562501 100644 --- a/hw/s390x/sclpconsole.c +++ b/hw/s390x/sclpconsole.c @@ -14,6 +14,7 @@ #include #include "qemu/thread.h" +#include "qemu/error-report.h" #include "sclp.h" #include "event-facility.h" diff --git a/hw/usb/de
[Qemu-devel] [PATCH 04/10] hw/arm_sysctl: Implement SYS_CFG_DVIMODE as a no-op
SYS_CFG_DVIMODE allows the guest to select whether the
output DVI signal is VGA, SVGA, XGA, SGA or UXGA. Since
this makes no difference to QEMU, implement writes as a
no-op so Linux doesn't complain.
Signed-off-by: Peter Maydell
---
hw/arm_sysctl.c |8
1 file changed, 8 insertions(+)
diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
index 960b664..bb56238 100644
--- a/hw/arm_sysctl.c
+++ b/hw/arm_sysctl.c
@@ -295,6 +295,14 @@ static bool vexpress_cfgctrl_write(arm_sysctl_state *s,
unsigned int dcc,
return true;
}
break;
+case SYS_CFG_DVIMODE:
+if (site == SYS_CFG_SITE_MB && device == 0) {
+/* Selecting DVI mode is meaningless for QEMU: we will
+ * always display the output correctly according to the
+ * pixel height/width programmed into the CLCD controller.
+ */
+return true;
+}
default:
break;
}
--
1.7.9.5
[Qemu-devel] [PATCH 6/8] vl.c: Use parse_uint_full() for NUMA nodeid
This should catch many kinds of errors that the current code wasn't
checking for:
- Values that can't be parsed as a number
- Negative values
- Overflow
- Empty string
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 4955c29..d6f6422 100644
--- a/vl.c
+++ b/vl.c
@@ -1267,7 +1267,10 @@ static void numa_add(const char *optarg)
if (get_param_value(option, 128, "nodeid", optarg) == 0) {
nodenr = nb_numa_nodes;
} else {
-nodenr = strtoull(option, NULL, 10);
+if (parse_uint_full(option, &nodenr, 10) < 0) {
+fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
+exit(1);
+}
}
if (nodenr >= MAX_NODES) {
--
1.8.1
[Qemu-devel] [PATCH for-1.4 0/8] -numa option parsing fixes (v7)
Resending whole series after v6 was reviewed and got no objections. Changes v6 -> v7: - Refresh of patch 1/8 against qemu.git master (trivial conflicts on tests/Makefile) - Correction of typo on comment on util/cutils.c Eduardo Habkost (8): cutils: unsigned int parsing functions vl.c: Fix off-by-one bug when handling "-numa node" argument vl.c: Abort on unknown -numa option type vl.c: Check for NUMA node limit inside numa_add() vl.c: numa_add(): Validate nodeid before using it vl.c: Use parse_uint_full() for NUMA nodeid vl.c: Extract -numa "cpus" parsing to separate function vl.c: validate -numa "cpus" parameter properly include/qemu-common.h | 4 + tests/Makefile| 3 + tests/test-cutils.c | 251 ++ util/cutils.c | 99 vl.c | 93 ++- 5 files changed, 425 insertions(+), 25 deletions(-) create mode 100644 tests/test-cutils.c -- 1.8.1
[Qemu-devel] [PATCH 8/8] vl.c: validate -numa "cpus" parameter properly
- Accept empty strings without aborting
- Use parse_uint*() to parse numbers
- Abort if anything except '-' or end-of-string is found after the first
number.
- Check for endvalue < value
Also change the MAX_CPUMASK_BITS warning message from "A max of %d CPUs
are supported in a guest" to "qemu: NUMA: A max of %d VCPUs are
supported".
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 32 +++-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/vl.c b/vl.c
index de164f8..a8dc73d 100644
--- a/vl.c
+++ b/vl.c
@@ -1249,21 +1249,43 @@ static void numa_node_parse_cpus(int nodenr, const char
*cpus)
char *endptr;
unsigned long long value, endvalue;
-value = strtoull(cpus, &endptr, 10);
+/* Empty CPU range strings will be considered valid, they will simply
+ * not set any bit in the CPU bitmap.
+ */
+if (!*cpus) {
+return;
+}
+
+if (parse_uint(cpus, &value, &endptr, 10) < 0) {
+goto error;
+}
if (*endptr == '-') {
-endvalue = strtoull(endptr+1, &endptr, 10);
-} else {
+if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
+goto error;
+}
+} else if (*endptr == '\0') {
endvalue = value;
+} else {
+goto error;
}
-if (!(endvalue < MAX_CPUMASK_BITS)) {
+if (endvalue >= MAX_CPUMASK_BITS) {
endvalue = MAX_CPUMASK_BITS - 1;
fprintf(stderr,
-"A max of %d CPUs are supported in a guest\n",
+"qemu: NUMA: A max of %d VCPUs are supported\n",
MAX_CPUMASK_BITS);
}
+if (endvalue < value) {
+goto error;
+}
+
bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+return;
+
+error:
+fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
+exit(1);
}
static void numa_add(const char *optarg)
--
1.8.1
[Qemu-devel] [PATCH 18/19] hw: remove CPU dependencies
Some devices or headers are using poisoned identifiers.
For .c files, some are useless and we remove them.
For headers, wrap the definitions in the headers with
Signed-off-by: Paolo Bonzini
---
hw/char/etraxfs_ser.c|2 --
hw/char/sclpconsole.c|2 +-
hw/display/exynos4210_fimd.c |1 -
hw/pci/host-apb.c|8
include/hw/arm/exynos4210.h | 10 +++---
include/hw/arm/omap.h|4
include/hw/arm/pxa.h | 12 ++--
include/hw/cris/etraxfs.h|2 ++
include/hw/m68k/mcf.h|4
include/hw/misc/pcmcia.h |1 +
include/hw/pci/q35.h |1 -
include/hw/ppc/ppc.h |6 ++
include/hw/ppc/ppc4xx.h | 13 +++--
include/hw/sh4/sh.h |2 ++
include/hw/sparc/sun4m.h |2 ++
15 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/hw/char/etraxfs_ser.c b/hw/char/etraxfs_ser.c
index 7e24d34..b7499d7 100644
--- a/hw/char/etraxfs_ser.c
+++ b/hw/char/etraxfs_ser.c
@@ -78,7 +78,6 @@ static uint64_t
ser_read(void *opaque, hwaddr addr, unsigned int size)
{
struct etrax_serial *s = opaque;
-D(CPUCRISState *env = s->env);
uint32_t r = 0;
addr >>= 2;
@@ -116,7 +115,6 @@ ser_write(void *opaque, hwaddr addr,
struct etrax_serial *s = opaque;
uint32_t value = val64;
unsigned char ch = val64;
-D(CPUCRISState *env = s->env);
D(qemu_log("%s " TARGET_FMT_plx "=%x\n", __func__, addr, value));
addr >>= 2;
diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index 01ee76b..673db04 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -237,7 +237,7 @@ static int write_event_data(SCLPEvent *event,
EventBufferHeader *evt_buf_hdr)
return rc;
}
-static void trigger_ascii_console_data(void *env, int n, int level)
+static void trigger_ascii_console_data(void *s, int n, int level)
{
sclp_service_interrupt(0);
}
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 501dc5c..0c7aef9 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -23,7 +23,6 @@
*/
#include "qemu-common.h"
-#include "exec/cpu-all.h"
#include "hw/sysbus.h"
#include "ui/console.h"
#include "ui/pixel_ops.h"
diff --git a/hw/pci/host-apb.c b/hw/pci/host-apb.c
index 672a9fc..c7004d1 100644
--- a/hw/pci/host-apb.c
+++ b/hw/pci/host-apb.c
@@ -92,7 +92,7 @@ static void apb_config_writel (void *opaque, hwaddr addr,
{
APBState *s = opaque;
-APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
addr, val);
+APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
addr, val);
switch (addr & 0x) {
case 0x30 ... 0x4f: /* DMA error registers */
@@ -201,7 +201,7 @@ static uint64_t apb_config_readl (void *opaque,
val = 0;
break;
}
-APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, val);
+APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, val);
return val;
}
@@ -218,7 +218,7 @@ static void apb_pci_config_write(void *opaque, hwaddr addr,
APBState *s = opaque;
val = qemu_bswap_len(val, size);
-APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
addr, val);
+APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
addr, val);
pci_data_write(s->bus, addr, val, size);
}
@@ -230,7 +230,7 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr
addr,
ret = pci_data_read(s->bus, addr, size);
ret = qemu_bswap_len(ret, size);
-APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret);
+APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
return ret;
}
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index bb9a1dd..8798cbe 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -84,7 +84,10 @@ typedef struct Exynos4210Irq {
qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
} Exynos4210Irq;
-typedef struct Exynos4210State {
+typedef struct Exynos4210State Exynos4210State;
+
+#ifdef NEED_CPU_H
+struct Exynos4210State {
ARMCPU *cpu[EXYNOS4210_NCPUS];
Exynos4210Irq irqs;
qemu_irq *irq_table;
@@ -98,16 +101,17 @@ typedef struct Exynos4210State {
MemoryRegion boot_secondary;
MemoryRegion bootreg_mem;
i2c_bus *i2c_if[EXYNOS4210_I2C_NUMBER];
-} Exynos4210State;
+};
void exynos4210_write_secondary(ARMCPU *cpu,
const struct arm_boot_info *info);
+#endif
Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
unsigned long ram_size);
/* Initialize exynos4210 IRQ subsystem stub */
-qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
+qemu_irq *exynos4210_init_irq(Exynos4210Irq *s);
/* Initialize board IRQs.
* These IRQs contain splitted Int/External Combiner and External Gic IRQs */
diff --git a/include/hw/arm/omap.h b/incl
[Qemu-devel] [PATCH 4/4] virtio: console: add flow control
The virtio-serial-bus already has the logic to make flow control work
properly. Hook into the char layer's new ability to signal a backend is
writable again.
Signed-off-by: Amit Shah
---
hw/virtio-console.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 46072a0..0cf6072 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -20,6 +20,18 @@ typedef struct VirtConsole {
CharDriverState *chr;
} VirtConsole;
+/*
+ * Callback function that's called from chardevs when backend becomes
+ * writable.
+ */
+static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
+void *opaque)
+{
+VirtConsole *vcon = opaque;
+
+virtio_serial_throttle_port(&vcon->port, false);
+return false;
+}
/* Callback function that's called when the guest sends us data */
static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t
len)
@@ -48,6 +60,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const
uint8_t *buf, size_t len)
* do_flush_queued_data().
*/
ret = 0;
+qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked, vcon);
}
return ret;
}
--
1.8.1
[Qemu-devel] [PATCH 07/60] cpu_physical_memory_write_rom() needs to do TB invalidates
From: David Gibson
cpu_physical_memory_write_rom(), despite the name, can also be used to
write images into RAM - and will often be used that way if the machine
uses load_image_targphys() into RAM addresses.
However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw()
doesn't invalidate any cached TBs which might be affected by the region
written.
This was breaking reset (under full emu) on the pseries machine - we loaded
our firmware image into RAM, and while executing it rewrite the code at
the entry point (correctly causing a TB invalidate/refresh). When we
reset the firmware image was reloaded, but the TB from the rewrite was
still active and caused us to get an illegal instruction trap.
This patch fixes the bug by duplicating the tb invalidate code from
cpu_physical_memory_rw() in cpu_physical_memory_write_rom().
Signed-off-by: David Gibson
Signed-off-by: Anthony Liguori
(cherry picked from commit 0b57e287138728f72d88b06e69b970c5d745c44a)
Signed-off-by: Michael Tokarev
---
exec.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/exec.c b/exec.c
index 0a67f07..8e97f93 100644
--- a/exec.c
+++ b/exec.c
@@ -3625,6 +3625,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t
addr,
/* ROM/RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
+if (!cpu_physical_memory_is_dirty(addr1)) {
+/* invalidate code */
+tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
+/* set dirty bit */
+cpu_physical_memory_set_dirty_flags(
+addr1, (0xff & ~CODE_DIRTY_FLAG));
+}
qemu_put_ram_ptr(ptr);
}
len -= l;
--
1.7.10.4
[Qemu-devel] [PATCH v3 04/10] slirp: slirp/slirp.c coding style cleanup
The slirp glue code uses tabs in some places. Since the next patch will
modify the file, convert tabs to spaces and fix checkpatch.pl issues.
Signed-off-by: Stefan Hajnoczi
---
slirp/slirp.c | 608 ++
1 file changed, 311 insertions(+), 297 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 0e6e232..5d14e7f 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -287,135 +287,139 @@ void slirp_select_fill(int *pnfds,
global_xfds = NULL;
nfds = *pnfds;
- /*
-* First, TCP sockets
-*/
- do_slowtimo = 0;
-
- QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
- /*
-* *_slowtimo needs calling if there are IP fragments
-* in the fragment queue, or there are TCP connections active
-*/
- do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
- (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
-
- for (so = slirp->tcb.so_next; so != &slirp->tcb;
-so = so_next) {
- so_next = so->so_next;
-
- /*
-* See if we need a tcp_fasttimo
-*/
- if (time_fasttimo == 0 && so->so_tcpcb->t_flags &
TF_DELACK)
- time_fasttimo = curtime; /* Flag when we want a
fasttimo */
-
- /*
-* NOFDREF can include still connecting to local-host,
-* newly socreated() sockets etc. Don't want to select
these.
-*/
- if (so->so_state & SS_NOFDREF || so->s == -1)
- continue;
-
- /*
-* Set for reading sockets which are accepting
-*/
- if (so->so_state & SS_FACCEPTCONN) {
-FD_SET(so->s, readfds);
- UPD_NFDS(so->s);
- continue;
- }
-
- /*
-* Set for writing sockets which are connecting
-*/
- if (so->so_state & SS_ISFCONNECTING) {
- FD_SET(so->s, writefds);
- UPD_NFDS(so->s);
- continue;
- }
-
- /*
-* Set for writing if we are connected, can send more,
and
-* we have something to send
-*/
- if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
- FD_SET(so->s, writefds);
- UPD_NFDS(so->s);
- }
-
- /*
-* Set for reading (and urgent data) if we are
connected, can
-* receive more, and we have room for it XXX /2 ?
-*/
- if (CONN_CANFRCV(so) && (so->so_snd.sb_cc <
(so->so_snd.sb_datalen/2))) {
- FD_SET(so->s, readfds);
- FD_SET(so->s, xfds);
- UPD_NFDS(so->s);
- }
- }
-
- /*
-* UDP sockets
-*/
- for (so = slirp->udb.so_next; so != &slirp->udb;
-so = so_next) {
- so_next = so->so_next;
-
- /*
-* See if it's timed out
-*/
- if (so->so_expire) {
- if (so->so_expire <= curtime) {
- udp_detach(so);
- continue;
- } else
- do_slowtimo = 1; /* Let socket expire */
- }
-
- /*
-* When UDP packets are received from over the
-* link, they're sendto()'d straight away, so
-* no need for setting for writing
-* Limit the number of packets queued by this session
-* to 4. Note that even though we try and limit this
-* to 4 packets, the session could have more queued
-* if the packets needed to be fragmented
-* (XXX <= 4 ?)
-*/
- if ((so->so_state & SS_ISFCONNECTED) && so->so_queued
<= 4) {
- FD_SET(so->s, readfds);
- UPD_NFDS(so->s);
- }
- }
+/*
+ * First, TCP sockets
+ */
+do_slowtimo = 0;
-
[Qemu-devel] [PATCH 17/19] hw: use _p variants, not _raw
ld*_raw and st*_raw is not available in common-obj-y, but it is also useless in softmmu targets: what it does is this #define laddr(x) (uint8_t *)(intptr_t)(x) #define ldub_raw(p) ldub_p(laddr((p))) #define ldsb_raw(p) ldsb_p(laddr((p))) #define lduw_raw(p) lduw_p(laddr((p))) #define ldsw_raw(p) ldsw_p(laddr((p))) ... in case the pointer is a target_ulong. Using ld*_p is more type-safe and will let us compile some devices once only. Signed-off-by: Paolo Bonzini --- hw/9pfs/virtio-9p-device.c|2 +- hw/arm/nseries.c | 236 hw/block/virtio-blk.c | 12 +- hw/display/blizzard_template.h|2 +- hw/display/exynos4210_fimd.c |4 +- hw/display/milkymist-vgafb_template.h |2 +- hw/display/omap_lcd_template.h| 10 +- hw/display/sm501_template.h |6 +- hw/display/vga_template.h |4 +- hw/mips/mips_fulong2e.c | 28 ++-- hw/mips/mips_malta.c | 176 hw/scsi/virtio-scsi.c | 28 ++-- 12 files changed, 255 insertions(+), 255 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 9eab65a..22c10f1 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -39,7 +39,7 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config) len = strlen(s->tag); cfg = g_malloc0(sizeof(struct virtio_9p_config) + len); -stw_raw(&cfg->tag_len, len); +stw_p(&cfg->tag_len, len); /* We don't copy the terminating null to config space */ memcpy(cfg->tag, s->tag, len); memcpy(config, cfg, s->config_size); diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index 3b6943a..c94ab28 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -840,118 +840,118 @@ static void n800_setup_nolo_tags(void *sram_base) strcpy((void *) (p + 8), "F5"); -stl_raw(p + 10, 0x04f7); +stl_p(p + 10, 0x04f7); strcpy((void *) (p + 9), "RX-34"); /* RAM size in MB? */ -stl_raw(p + 12, 0x80); +stl_p(p + 12, 0x80); /* Pointer to the list of tags */ -stl_raw(p + 13, OMAP2_SRAM_BASE + 0x9000); +stl_p(p + 13, OMAP2_SRAM_BASE + 0x9000); /* The NOLO tags start here */ p = sram_base + 0x9000; #define ADD_TAG(tag, len) \ -stw_raw((uint16_t *) p + 0, tag); \ -stw_raw((uint16_t *) p + 1, len); p ++;\ -stl_raw(p ++, OMAP2_SRAM_BASE | (((void *) v - sram_base) & 0x)); +stw_p((uint16_t *) p + 0, tag);\ +stw_p((uint16_t *) p + 1, len); p ++; \ +stl_p(p ++, OMAP2_SRAM_BASE | (((void *) v - sram_base) & 0x)); /* OMAP STI console? Pin out settings? */ ADD_TAG(0x6e01, 414); for (i = 0; i < ARRAY_SIZE(n800_pinout); i ++) -stl_raw(v ++, n800_pinout[i]); +stl_p(v ++, n800_pinout[i]); /* Kernel memsize? */ ADD_TAG(0x6e05, 1); -stl_raw(v ++, 2); +stl_p(v ++, 2); /* NOLO serial console */ ADD_TAG(0x6e02, 4); -stl_raw(v ++, XLDR_LL_UART); /* UART number (1 - 3) */ +stl_p(v ++, XLDR_LL_UART); /* UART number (1 - 3) */ #if 0 /* CBUS settings (Retu/AVilma) */ ADD_TAG(0x6e03, 6); -stw_raw((uint16_t *) v + 0, 65); /* CBUS GPIO0 */ -stw_raw((uint16_t *) v + 1, 66); /* CBUS GPIO1 */ -stw_raw((uint16_t *) v + 2, 64); /* CBUS GPIO2 */ +stw_p((uint16_t *) v + 0, 65); /* CBUS GPIO0 */ +stw_p((uint16_t *) v + 1, 66); /* CBUS GPIO1 */ +stw_p((uint16_t *) v + 2, 64); /* CBUS GPIO2 */ v += 2; #endif /* Nokia ASIC BB5 (Retu/Tahvo) */ ADD_TAG(0x6e0a, 4); -stw_raw((uint16_t *) v + 0, 111); /* "Retu" interrupt GPIO */ -stw_raw((uint16_t *) v + 1, 108); /* "Tahvo" interrupt GPIO */ +stw_p((uint16_t *) v + 0, 111);/* "Retu" interrupt GPIO */ +stw_p((uint16_t *) v + 1, 108);/* "Tahvo" interrupt GPIO */ v ++; /* LCD console? */ ADD_TAG(0x6e04, 4); -stw_raw((uint16_t *) v + 0, 30); /* ??? */ -stw_raw((uint16_t *) v + 1, 24); /* ??? */ +stw_p((uint16_t *) v + 0, 30); /* ??? */ +stw_p((uint16_t *) v + 1, 24); /* ??? */ v ++; #if 0 /* LCD settings */ ADD_TAG(0x6e06, 2); -stw_raw((uint16_t *) (v ++), 15); /* ??? */ +stw_p((uint16_t *) (v ++), 15);/* ??? */ #endif /* I^2C (Menelaus) */ ADD_TAG(0x6e07, 4); -stl_raw(v ++, 0x0072); /* ??? */ +stl_p(v ++, 0x0072); /* ??? */ /* Unknown */ ADD_TAG(0x6e0b, 6); -stw_raw((uint16_t *) v + 0, 94); /* ??? */ -stw_raw((uint16_t *) v + 1, 23); /* ??? */ -stw_raw((uint16_t *) v + 2, 0);/* ??? */ +stw_p((uint16_t *) v + 0, 94); /* ??? */ +stw_p((uint16_t *) v + 1, 23); /* ??? */ +stw_p((uint16_t *) v + 2
Re: [Qemu-devel] [PATCH] check-qjson: More thorough testing of UTF-8 in strings
On Mon, Feb 4, 2013 at 5:19 PM, Markus Armbruster wrote:
> Test cases are scraped from Markus Kuhn's UTF-8 decoder capability and
> stress test at
> http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
There's no license. Can we use that?
>
> Unfortunately, both JSON parser and formatter misbehave right now.
> This test expects current, incorrect results. They're all clearly
> marked, and are to be replaced by correct ones as the bugs get fixed.
> See comments in new utf8_string() for details.
This clearly calls for another fuzz test.
>
> Signed-off-by: Markus Armbruster
> ---
> tests/check-qjson.c | 625
>
> 1 file changed, 625 insertions(+)
>
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 32ffb43..4590b3a 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -1,8 +1,10 @@
> /*
> * Copyright IBM, Corp. 2009
> + * Copyright (c) 2013 Red Hat Inc.
> *
> * Authors:
> * Anthony Liguori
> + * Markus Armbruster ,
> *
> * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> * See the COPYING.LIB file in the top-level directory.
> @@ -131,6 +133,628 @@ static void single_quote_string(void)
> }
> }
>
> +static void utf8_string(void)
> +{
> +/*
> + * FIXME Current behavior for invalid UTF-8 sequences is
> + * incorrect. This test expects current, incorrect results.
> + * They're all marked "bug:" below, and are to be replaced by
> + * correct ones as the bugs get fixed.
> + *
> + * The JSON parser rejects some invalid sequences, but accepts
> + * others without correcting the problem.
> + *
> + * The JSON formatter replaces some invalid sequences by U+ (a
> + * noncharacter), and goes wonky for others.
> + *
> + * For both directions, we should either reject all invalid
> + * sequences, or minimize overlong sequences and replace all other
> + * invalid sequences by a suitable replacement character. A
> + * common choice for replacement is U+FFFD.
> + *
> + * Problem: we can't easily deal with embedded U+. Parsing
> + * the JSON string "this \\u" is fun" yields "this \0 is fun",
> + * which gets misinterpreted as NUL-terminated "this ". We should
> + * consider using overlong encoding \xC0\x80 for U+ ("modified
> + * UTF-8").
> + *
> + * Tests are scraped from Markus Kuhn's UTF-8 decoder capability
> + * and stress test at
> + * http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
> + */
> +static const struct {
> +const char *json_in;
> +const char *utf8_out;
> +const char *json_out; /* defaults to @json_in */
> +const char *utf8_in;/* defaults to @utf8_out */
> +} test_cases[] = {
> +/*
> + * Bug markers used here:
> + * - bug: not corrected
> + * JSON parser fails to correct invalid sequence(s)
> + * - bug: rejected
> + * JSON parser rejects invalid sequence(s)
> + * We may choose to define this as feature
> + * - bug: want "\"...\""
> + * JSON formatter produces incorrect result, this is the
> + * correct one, assuming replacement character U+
> + * - bug: want "..." (no \")
> + * JSON parser produces incorrect result, this is the
> + * correct one.
> + * Not marked explicitly, but trivial to find:
> + * - JSON formatter replacing invalid sequence by \\u is a
> + * bug if we want it to fail for invalid sequences.
> + */
> +
> +/* 1 Some correct UTF-8 text */
> +{
> +/* a bit of German */
> +"\"Falsches \xC3\x9C" "ben von Xylophonmusik qu\xC3\xA4lt"
> +" jeden gr\xC3\xB6\xC3\x9F" "eren Zwerg.\"",
> +"Falsches \xC3\x9C" "ben von Xylophonmusik qu\xC3\xA4lt"
> +" jeden gr\xC3\xB6\xC3\x9F" "eren Zwerg.",
> +"\"Falsches \\u00DCben von Xylophonmusik qu\\u00E4lt"
> +" jeden gr\\u00F6\\u00DFeren Zwerg.\"",
> +},
> +{
> +/* a bit of Greek */
> +"\"\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5\"",
> +"\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
> +"\"\\u03BA\\u1F79\\u03C3\\u03BC\\u03B5\"",
> +},
> +/* 2 Boundary condition test cases */
> +/* 2.1 First possible sequence of a certain length */
> +/* 2.1.1 1 byte U+ */
> +{
> +"\"\\u\"",
> +"", /* bug: want overlong "\xC0\x80" */
> +"\"\"", /* bug: want "\"\\u\"" */
> +},
> +/* 2.1.2 2 bytes U+0080 */
> +{
> +"\"\xC2\x80\"",
> +"\xC2\x80",
> +"\"\\u0080\"",
> +},
> +/* 2.1.3 3 bytes U+0800 */
> +{
> +"\"\xE0\xA0\x8
Re: [Qemu-devel] [PATCH 00/19] hw/ directory restructuring
Il 04/02/2013 19:13, Peter Maydell ha scritto: >> > This reorganizes the over 600 files in the hw/ directory to multiple >> > subdirectories: acpi/, audio/, block/, bt/, char/, display/, i2c/, input/, >> > isa/, misc/, net/, scsi/, sd/, ssi/, timer/, virtio/, watchdog/, xen/. >> > These are in addition to the existing 9pfs/, ide/, kvm/, pci/, usb/ >> > directories, and to the per-target directories. > I think this is inconsistent. Either we should organise by category, > or we should organise per-target, but we shouldn't try to do both. > Otherwise we're just going to wind up with half the $TARGET-specific > $FOO device models in hw/$TARGET and the other half in hw/$FOO. The $FOO device models should all be in hw/$FOO. But there are some parts that do not fit in a category or are always target-specific, and hw/$TARGET has them. Also, the kernel similarly has arch/$TARGET and drivers/$FOO. Paolo
[Qemu-devel] [PATCH 3/4] char: add gio watch fn for tcp backends
Signed-off-by: Amit Shah
---
qemu-char.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index 7fa9372..01b0190 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2484,6 +2484,12 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char
*buf, size_t len)
}
#endif
+static GSource *tcp_chr_add_watch(CharDriverState *chr, GIOCondition cond)
+{
+PtyCharDriver *s = chr->opaque;
+return g_io_create_watch(s->fd, cond);
+}
+
static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
@@ -2698,6 +2704,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd,
bool do_nodelay,
chr->chr_close = tcp_chr_close;
chr->get_msgfd = tcp_get_msgfd;
chr->chr_add_client = tcp_chr_add_client;
+chr->chr_add_watch = tcp_chr_add_watch;
if (is_listen) {
s->listen_fd = fd;
--
1.8.1
Re: [Qemu-devel] [PATCH 14/19] hw: use default-configs for more devices instead of hw/ARCH/Makefile.objs
Am 04.02.2013 18:29, schrieb Paolo Bonzini: > Other devices are now moved out of hw/ARCH, adding new CONFIG_* > symbols that let them be included selectively in the emulators. > The devices however are still compiled in target-specific > directories. > > Signed-off-by: Paolo Bonzini I don't think this is a good idea. For one thing the patch does too many movements at once for proper commenting. For another you group devices strictly by busses. And once again this invasive series does not seem to CC the respective maintainers whose code you move around. We just had a discussion in the Port I/O context that devices should not derive according to busses but should have-a bus-specific connector interface/object from a pure chipset object. That would contradict the split by bus IMO (but I wasn't the one to call for that). Also you are moving files like, e.g., sPAPR or ppc4xx into mst's pci/ directory, where he is rather unlikely to do much work on. It would be more sensible to have sPAPR PHB live alongside other sPAPR devices that David et al. maintain, say hw/ppc/spapr/. Apart from that I already commented on the USB movement that, e.g., spapr_pci is more natural to read than the reversed host-spapr. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH V20 6/8] Add support for cancelling of a TPM command
On 02/04/2013 11:02 AM, Corey Bryant wrote:
@@ -221,7 +243,24 @@ static void
tpm_passthrough_deliver_request(TPMBackend *tb)
static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
{
-/* cancelling an ongoing command is known not to work with some
TPMs */
+TPMPassthruState *tpm_pt = tb->s.tpm_pt;
+int n;
+
+/*
+ * As of Linux 3.7 the tpm_tis driver does not properly cancel
+ * commands for all TPMs.
Any idea what the plan is for this issue? Is it an ongoing matter of
adding kernel support as unsupported TPMs are identified?
I submitted a patch to the tpmdd-devel mailing list fixing the
cancellation issue with the particular TPM I have on one of my machines.
Kent Yoder modified it to add a fix for yet another TPM (from a
different vendor).
https://github.com/shpedoikal/linux/commit/9f11986de7280f999cad342389b48c29002c0f37
The spec seems not be clear enough as to what bits must be set in the
status register when a TPM command is canceled so that the so far
implemented cancellation detection is insufficient and needs to be
adapted to TPM vendors' implementations. All TIS interfaces are supposed
to support cancellation, though.
+ * Only cancel if we're busy so we don't cancel someone else's
+ * command, e.g., a command executed on the host.
+ */
+if (tpm_pt->cancel_fd >= 0 && tpm_pt->tpm_executing) {
+n = write(tpm_pt->cancel_fd, "-", 1);
+if (n != 1) {
+error_report("Canceling TPM command failed: %s\n",
+ strerror(errno));
+} else {
+tpm_pt->tpm_op_canceled = true;
+}
+}
Would an informational message make sense here for unsupported TPMs
(when tpm_pt->cancel_fd < 0)?
Sure, I can add that.
+snprintf(path, sizeof(path), "/sys/class/misc/tpm%u/device",
i);
+if (!tpm_passthrough_check_sysfs_cancel(path, sizeof(path))) {
+continue;
+}
+fd = open(path, O_WRONLY);
+break;
+}
+
+exit:
+if (fd >= 0) {
+tb->cancel_path = g_strdup(path);
+}
+
+return fd;
+}
+
A general question about the function above. I see that
"/sys/class/misc/tpm%u/device" will be explained in kernel
documentation here:
https://github.com/shpedoikal/linux/commit/4e21d66c9efbe740b5655bcf66e39873e354f8e9
And the following paths apparently have the same behavior. But are
these paths defined somewhere else?
"/sys/devices/pnp%u/%s"
"/sys/devices/platform/tpm_tis"
Also I think a comment pointing to documentation would be a
worth-while addition to the code.
The /sys/class/misc/tpm%u/device path seems to be always there
independent of what type of device the TPM is registered as (pnp or
platform). So I'll modify above code to always use that path instead.
Stefan
[Qemu-devel] [PATCH 05/19] virtio-9p: use CONFIG_VIRTFS, not CONFIG_LINUX
Signed-off-by: Paolo Bonzini
---
hw/virtio-pci.h |2 +-
hw/virtio.h |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index d24957c..e775525 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -75,7 +75,7 @@ struct VirtIOPCIProxy {
VirtIOBlkConf blk;
NICConf nic;
uint32_t host_features;
-#ifdef CONFIG_LINUX
+#ifdef CONFIG_VIRTFS
V9fsConf fsconf;
#endif
virtio_serial_conf serial;
diff --git a/hw/virtio.h b/hw/virtio.h
index a29a54d..e3c7aa2 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -19,7 +19,7 @@
#include "qdev.h"
#include "sysemu/sysemu.h"
#include "qemu/event_notifier.h"
-#ifdef CONFIG_LINUX
+#ifdef CONFIG_VIRTFS
#include "9p.h"
#endif
@@ -251,7 +251,7 @@ typedef struct VirtIOSCSIConf VirtIOSCSIConf;
VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *conf);
typedef struct VirtIORNGConf VirtIORNGConf;
VirtIODevice *virtio_rng_init(DeviceState *dev, VirtIORNGConf *conf);
-#ifdef CONFIG_LINUX
+#ifdef CONFIG_VIRTFS
VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
#endif
--
1.7.4.1
Re: [Qemu-devel] [PATCH 18/19] hw: remove CPU dependencies
Am 04.02.2013 18:30, schrieb Paolo Bonzini:
> Some devices or headers are using poisoned identifiers.
> For .c files, some are useless and we remove them.
>
> For headers, wrap the definitions in the headers with
#if at start of line gets dropped by git. ;)
>
> Signed-off-by: Paolo Bonzini
> ---
> hw/char/etraxfs_ser.c|2 --
> hw/char/sclpconsole.c|2 +-
> hw/display/exynos4210_fimd.c |1 -
> hw/pci/host-apb.c|8
> include/hw/arm/exynos4210.h | 10 +++---
> include/hw/arm/omap.h|4
> include/hw/arm/pxa.h | 12 ++--
> include/hw/cris/etraxfs.h|2 ++
> include/hw/m68k/mcf.h|4
> include/hw/misc/pcmcia.h |1 +
> include/hw/pci/q35.h |1 -
> include/hw/ppc/ppc.h |6 ++
> include/hw/ppc/ppc4xx.h | 13 +++--
> include/hw/sh4/sh.h |2 ++
> include/hw/sparc/sun4m.h |2 ++
> 15 files changed, 50 insertions(+), 20 deletions(-)
Again this is too much at once, please split by maintainer or by file.
If you ordered this first, so that it applies to qemu.git file
locations, some of these could get applied through the respective
maintainers already.
> diff --git a/hw/char/etraxfs_ser.c b/hw/char/etraxfs_ser.c
> index 7e24d34..b7499d7 100644
> --- a/hw/char/etraxfs_ser.c
> +++ b/hw/char/etraxfs_ser.c
> @@ -78,7 +78,6 @@ static uint64_t
> ser_read(void *opaque, hwaddr addr, unsigned int size)
> {
> struct etrax_serial *s = opaque;
> -D(CPUCRISState *env = s->env);
> uint32_t r = 0;
>
> addr >>= 2;
> @@ -116,7 +115,6 @@ ser_write(void *opaque, hwaddr addr,
> struct etrax_serial *s = opaque;
> uint32_t value = val64;
> unsigned char ch = val64;
> -D(CPUCRISState *env = s->env);
>
> D(qemu_log("%s " TARGET_FMT_plx "=%x\n", __func__, addr, value));
> addr >>= 2;
Did you check compiling this with debug output macros enabled?
> diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
> index 01ee76b..673db04 100644
> --- a/hw/char/sclpconsole.c
> +++ b/hw/char/sclpconsole.c
> @@ -237,7 +237,7 @@ static int write_event_data(SCLPEvent *event,
> EventBufferHeader *evt_buf_hdr)
> return rc;
> }
>
> -static void trigger_ascii_console_data(void *env, int n, int level)
> +static void trigger_ascii_console_data(void *s, int n, int level)
opaque might be a better name for a void *, allowing it to be cast to
SomeState *s later. (I tripped over so subtle name issues for CPUState)
> {
> sclp_service_interrupt(0);
> }
> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> index 501dc5c..0c7aef9 100644
> --- a/hw/display/exynos4210_fimd.c
> +++ b/hw/display/exynos4210_fimd.c
> @@ -23,7 +23,6 @@
> */
>
> #include "qemu-common.h"
> -#include "exec/cpu-all.h"
> #include "hw/sysbus.h"
> #include "ui/console.h"
> #include "ui/pixel_ops.h"
> diff --git a/hw/pci/host-apb.c b/hw/pci/host-apb.c
> index 672a9fc..c7004d1 100644
> --- a/hw/pci/host-apb.c
> +++ b/hw/pci/host-apb.c
> @@ -92,7 +92,7 @@ static void apb_config_writel (void *opaque, hwaddr addr,
> {
> APBState *s = opaque;
>
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
> addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
> addr, val);
>
> switch (addr & 0x) {
> case 0x30 ... 0x4f: /* DMA error registers */
> @@ -201,7 +201,7 @@ static uint64_t apb_config_readl (void *opaque,
> val = 0;
> break;
> }
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, val);
>
> return val;
> }
> @@ -218,7 +218,7 @@ static void apb_pci_config_write(void *opaque, hwaddr
> addr,
> APBState *s = opaque;
>
> val = qemu_bswap_len(val, size);
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %" PRIx64 "\n", __func__,
> addr, val);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__,
> addr, val);
> pci_data_write(s->bus, addr, val, size);
> }
>
> @@ -230,7 +230,7 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr
> addr,
>
> ret = pci_data_read(s->bus, addr, size);
> ret = qemu_bswap_len(ret, size);
> -APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret);
> +APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
> return ret;
> }
>
> diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
> index bb9a1dd..8798cbe 100644
> --- a/include/hw/arm/exynos4210.h
> +++ b/include/hw/arm/exynos4210.h
> @@ -84,7 +84,10 @@ typedef struct Exynos4210Irq {
> qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
> } Exynos4210Irq;
>
> -typedef struct Exynos4210State {
> +typedef struct Exynos4210State Exynos4210State;
> +
> +#ifdef NEED_CPU_H
> +struct Exynos4210S
Re: [Qemu-devel] [PATCH] bitops: unify bitops_flsl with host-utils.h, call it bitops_clzl
On 4 February 2013 18:26, Richard Henderson wrote:
> /**
> - * bitops_fls - find last (most-significant) set bit in a long word
> + * bitops_clzl - find last (most-significant) set bit in a long word
Should probably change the descriptive text to say "count leading
zeros"...
> * @word: the word to search
> *
> * Undefined if no set bit exists, so code should check against 0 first.
Should we change this to match clz32()/clz64()/bitops_ctzl() in not
having an undefined-behaviour case?
> */
> -static inline unsigned long bitops_flsl(unsigned long word)
> +static inline unsigned long bitops_clzl(unsigned long word)
> {
> - int num = BITS_PER_LONG - 1;
> -
> -#if LONG_MAX > 0x7FFF
> - if (!(word & (~0ul << 32))) {
> - num -= 32;
> - word <<= 32;
> - }
> +/* Both this function and the gcc builtin are undefined at 0, whereas
> + the host-utils functions return word-size at 0. Thus we prefer the
> + gcc builtin as the closer match. */
I think we should just fix the inconsistency rather than
retaining several similarly-named functions which have differing
corner-case behaviour.
-- PMM
Re: [Qemu-devel] [PATCH] bitops: unify bitops_flsl with host-utils.h, call it bitops_clzl
On 2013-02-04 10:34, Peter Maydell wrote: - } +/* Both this function and the gcc builtin are undefined at 0, whereas + the host-utils functions return word-size at 0. Thus we prefer the + gcc builtin as the closer match. */ I think we should just fix the inconsistency rather than retaining several similarly-named functions which have differing corner-case behaviour. As opposed to bitops_ctzl (-1) and host-utils ctzN (N) ? r~
Re: [Qemu-devel] [PATCH] linux-user: Support setgroups syscall with no groups
On 02/02/2013 04:04 PM, dill...@dillona.com wrote:
> From: Dillon Amburgey
>
> Signed-off-by: Dillon Amburgey
> ---
> linux-user/syscall.c | 22 --
> 1 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a148d9f..7344052 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7653,18 +7653,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> {
> int gidsetsize = arg1;
> target_id *target_grouplist;
> -gid_t *grouplist;
> +gid_t *grouplist = NULL;
> int i;
> -
> -grouplist = alloca(gidsetsize * sizeof(gid_t));
> -target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize * 2,
> 1);
> -if (!target_grouplist) {
> -ret = -TARGET_EFAULT;
> -goto fail;
> +if (gidsetsize) {
> +grouplist = alloca(gidsetsize * sizeof(gid_t));
Is this alloca() safe, or are you risking stack overflow if the user
passes an extremely large arg1?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 06/10] iohandler: switch to GPollFD
Convert iohandler_select_fill() and iohandler_select_poll() to use
GPollFD instead of rfds/wfds/xfds.
Signed-off-by: Stefan Hajnoczi
---
include/qemu/main-loop.h | 4 ++--
iohandler.c | 40 ++--
main-loop.c | 4 ++--
3 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index e8059c3..0995288 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -297,8 +297,8 @@ void qemu_mutex_unlock_iothread(void);
/* internal interfaces */
void qemu_fd_register(int fd);
-void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set
*xfds);
-void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int
rc);
+void qemu_iohandler_fill(GArray *pollfds);
+void qemu_iohandler_poll(GArray *pollfds, int rc);
QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
void qemu_bh_schedule_idle(QEMUBH *bh);
diff --git a/iohandler.c b/iohandler.c
index 2523adc..ae2ef8f 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -39,6 +39,7 @@ typedef struct IOHandlerRecord {
void *opaque;
QLIST_ENTRY(IOHandlerRecord) next;
int fd;
+int pollfds_idx;
bool deleted;
} IOHandlerRecord;
@@ -78,6 +79,7 @@ int qemu_set_fd_handler2(int fd,
ioh->fd_read = fd_read;
ioh->fd_write = fd_write;
ioh->opaque = opaque;
+ioh->pollfds_idx = -1;
ioh->deleted = 0;
qemu_notify_event();
}
@@ -92,38 +94,56 @@ int qemu_set_fd_handler(int fd,
return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
}
-void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set
*xfds)
+void qemu_iohandler_fill(GArray *pollfds)
{
IOHandlerRecord *ioh;
QLIST_FOREACH(ioh, &io_handlers, next) {
+int events = 0;
+
if (ioh->deleted)
continue;
if (ioh->fd_read &&
(!ioh->fd_read_poll ||
ioh->fd_read_poll(ioh->opaque) != 0)) {
-FD_SET(ioh->fd, readfds);
-if (ioh->fd > *pnfds)
-*pnfds = ioh->fd;
+events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
}
if (ioh->fd_write) {
-FD_SET(ioh->fd, writefds);
-if (ioh->fd > *pnfds)
-*pnfds = ioh->fd;
+events |= G_IO_OUT | G_IO_ERR;
+}
+if (events) {
+GPollFD pfd = {
+.fd = ioh->fd,
+.events = events,
+};
+ioh->pollfds_idx = pollfds->len;
+g_array_append_val(pollfds, pfd);
+} else {
+ioh->pollfds_idx = -1;
}
}
}
-void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int
ret)
+void qemu_iohandler_poll(GArray *pollfds, int ret)
{
if (ret > 0) {
IOHandlerRecord *pioh, *ioh;
QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
-if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, readfds)) {
+int revents = 0;
+
+if (!ioh->deleted && ioh->pollfds_idx != -1) {
+GPollFD *pfd = &g_array_index(pollfds, GPollFD,
+ ioh->pollfds_idx);
+revents = pfd->revents;
+}
+
+if (!ioh->deleted && ioh->fd_read &&
+(revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
ioh->fd_read(ioh->opaque);
}
-if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, writefds))
{
+if (!ioh->deleted && ioh->fd_write &&
+(revents & (G_IO_OUT | G_IO_ERR))) {
ioh->fd_write(ioh->opaque);
}
diff --git a/main-loop.c b/main-loop.c
index 49e97ff..313f369 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -505,9 +505,9 @@ int main_loop_wait(int nonblocking)
slirp_update_timeout(&timeout);
slirp_pollfds_fill(gpollfds);
#endif
-qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
+qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
-qemu_iohandler_poll(&rfds, &wfds, &xfds, ret);
+qemu_iohandler_poll(gpollfds, ret);
#ifdef CONFIG_SLIRP
slirp_pollfds_poll(gpollfds, (ret < 0));
#endif
--
1.8.1
Re: [Qemu-devel] [PATCH v3 1/3] VFIO: Wrapper for getting reference to vfio_device from device
On Sun, 2013-02-03 at 14:10 +, Pandarathil, Vijaymohan R wrote:
> - Added vfio_device_get_from_dev() as wrapper to get
> reference to vfio_device from struct device.
>
> - Added vfio_device_data() as a wrapper to get device_data from
> vfio_device.
>
> Signed-off-by: Vijay Mohan Pandarathil
> ---
> drivers/vfio/vfio.c | 41 -
> include/linux/vfio.h | 3 +++
> 2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 12c264d..f0a78a2 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -407,12 +407,13 @@ static void vfio_device_release(struct kref *kref)
> }
>
> /* Device reference always implies a group reference */
> -static void vfio_device_put(struct vfio_device *device)
> +void vfio_device_put(struct vfio_device *device)
> {
> struct vfio_group *group = device->group;
> kref_put_mutex(&device->kref, vfio_device_release, &group->device_lock);
> vfio_group_put(group);
> }
> +EXPORT_SYMBOL_GPL(vfio_device_put);
>
> static void vfio_device_get(struct vfio_device *device)
> {
> @@ -642,8 +643,12 @@ int vfio_add_group_dev(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(vfio_add_group_dev);
>
> -/* Test whether a struct device is present in our tracking */
> -static bool vfio_dev_present(struct device *dev)
> +/**
> + * This does a get on the vfio_device from device.
> + * Callers of this function will have to call vfio_put_device() to
> + * remove the reference.
> + */
> +struct vfio_device *vfio_device_get_from_dev(struct device *dev)
> {
> struct iommu_group *iommu_group;
> struct vfio_group *group;
> @@ -651,25 +656,43 @@ static bool vfio_dev_present(struct device *dev)
>
> iommu_group = iommu_group_get(dev);
> if (!iommu_group)
> - return false;
> + return NULL;
>
> group = vfio_group_get_from_iommu(iommu_group);
> if (!group) {
> iommu_group_put(iommu_group);
> - return false;
> + return NULL;
> }
>
> device = vfio_group_get_device(group, dev);
> if (!device) {
> vfio_group_put(group);
> iommu_group_put(iommu_group);
> - return false;
> + return NULL;
nit, this test isn't necesary, skipping the whole if block and falling
through to the return below is functionally the same.
> }
> -
> - vfio_device_put(device);
> vfio_group_put(group);
> iommu_group_put(iommu_group);
> - return true;
> + return device;
> +}
> +EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);
> +
Let's add a comment here that the caller must hold a reference to the
vfio_device. Thanks,
Alex
> +void *vfio_device_data(struct vfio_device *device)
> +{
> + return device->device_data;
> +}
> +EXPORT_SYMBOL_GPL(vfio_device_data);
> +
> +/* Test whether a struct device is present in our tracking */
> +static bool vfio_dev_present(struct device *dev)
> +{
> + struct vfio_device *device;
> +
> + device = vfio_device_get_from_dev(dev);
> + if (device) {
> + vfio_device_put(device);
> + return true;
> + } else
> + return false;
> }
>
> /*
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ab9e862..ac8d488 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -45,6 +45,9 @@ extern int vfio_add_group_dev(struct device *dev,
> void *device_data);
>
> extern void *vfio_del_group_dev(struct device *dev);
> +extern struct vfio_device *vfio_device_get_from_dev(struct device *dev);
> +extern void vfio_device_put(struct vfio_device *device);
> +extern void *vfio_device_data(struct vfio_device *device);
>
> /**
> * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
Re: [Qemu-devel] [PATCH 1/8] cutils: unsigned int parsing functions
On 02/04/13 19:27, Eduardo Habkost wrote: > Changes on v7 are trivial: > - Patch refresh after changes on test/Makefile on qemu.git master > - Fix typo on comment on parse_uint() (missing whitespace) > > As the changes are really trivial, I am keeping the Reviewed-by lines > from Eric and Laszlo. Thank goodness! :) Laszlo
Re: [Qemu-devel] [PATCH] bitops: unify bitops_flsl with host-utils.h, call it bitops_clzl
On 02/04/2013 11:26 AM, Richard Henderson wrote: > Similar to the bitops_ctz change just made. > > Signed-off-by: Richard Henderson > --- > include/qemu/bitops.h | 44 +++- > target-i386/topology.h | 2 +- > util/bitops.c | 2 +- > 3 files changed, 17 insertions(+), 31 deletions(-) Reviewed-by: Eric Blake > > diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h > index 8b88791..07ada63 100644 > --- a/include/qemu/bitops.h > +++ b/include/qemu/bitops.h > @@ -50,41 +50,27 @@ static unsigned long bitops_ctzl(unsigned long word) > } > > /** > - * bitops_fls - find last (most-significant) set bit in a long word > + * bitops_clzl - find last (most-significant) set bit in a long word > * @word: the word to search > * > * Undefined if no set bit exists, so code should check against 0 first. Interesting that you chose to leave 0 undefined here, even though bitops_ctzl specifically defines that case, but not a show-stopper. Per Paolo's arguments in the other thread, if we did decide to define bitops_clzl(0), setting it to the word size would be a reasonable choice (bitops_clzl(2)=>62, bitops_clzl(1)=>63, bitops_clzl(0)=>64), but that can be done later if it turns out to be useful. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv2] virtio-net: remove mq feature flag
On Mon, Feb 4, 2013 at 7:41 AM, Michael S. Tsirkin wrote:
> mq flag is not needed: we can look at the number of queues and set
> the flag accordingly.
> Removing this feature removes ambiguity (what does it mean to have
> queues=2 with mq=off?), and simplifies compatibility hacks.
> work-around for buggy windows
> guests.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> hw/pc_piix.c| 4
> hw/virtio-net.c | 8 +++-
> hw/virtio-net.h | 15 ---
> 3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 0af436c..ba09714 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -313,10 +313,6 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
> .driver = "virtio-net-pci",\
> .property = "ctrl_mac_addr",\
> .value= "off", \
> -},{ \
> -.driver = "virtio-net-pci", \
> -.property = "mq", \
> -.value= "off", \
> }
>
> static QEMUMachine pc_machine_v1_3 = {
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index e37358a..8db1787 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice
> *vdev, uint32_t features)
>
> features |= (1 << VIRTIO_NET_F_MAC);
>
> +if (n->max_queues > 1) {
> +features |= (0x1 << VIRTIO_NET_F_MQ);
> +}
> +
> if (!peer_has_vnet_hdr(n)) {
> features &= ~(0x1 << VIRTIO_NET_F_CSUM);
> features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
> @@ -1285,7 +1289,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf
> *conf,
> int i;
>
> n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
> -sizeof(struct virtio_net_config),
> +conf->queues > 1 ?
> +sizeof(struct virtio_net_config)
> :
> +sizeof(struct
> virtio_net_config_compat),
> sizeof(VirtIONet));
>
> n->vdev.get_config = virtio_net_get_config;
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index f5fea6e..b1f7647 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -69,6 +69,17 @@ typedef struct virtio_net_conf
> /* Maximum packet size we can receive from tap device: header + 64k */
> #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
>
> +/*
> + * Windows drivers from 22 Jan 2013 and older fail when config size != 32.
> + */
> +struct virtio_net_config_compat
VirtioNetConfigCompat, don't forget the typedef.
> +{
> +/* The config defining mac address ($ETH_ALEN bytes) */
> +uint8_t mac[ETH_ALEN];
> +/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
> +uint16_t status;
> +} QEMU_PACKED;
> +
> struct virtio_net_config
> {
> /* The config defining mac address ($ETH_ALEN bytes) */
> @@ -190,7 +201,5 @@ struct virtio_net_ctrl_mq {
> DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX,
> true), \
> DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN,
> true), \
> DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field,
> VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
> -DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field,
> VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
> -DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, true)
> -
> +DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field,
> VIRTIO_NET_F_CTRL_MAC_ADDR, true)
> #endif
> --
> MST
>
Re: [Qemu-devel] [PATCH 4/4] virtio: console: add flow control
On Mon, Feb 4, 2013 at 12:50 PM, Amit Shah wrote:
> The virtio-serial-bus already has the logic to make flow control work
> properly. Hook into the char layer's new ability to signal a backend is
> writable again.
>
> Signed-off-by: Amit Shah
> ---
> hw/virtio-console.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index 46072a0..0cf6072 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -20,6 +20,18 @@ typedef struct VirtConsole {
> CharDriverState *chr;
> } VirtConsole;
>
> +/*
> + * Callback function that's called from chardevs when backend becomes
> + * writable.
> + */
> +static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
> +void *opaque)
> +{
> +VirtConsole *vcon = opaque;
> +
> +virtio_serial_throttle_port(&vcon->port, false);
> +return false;
FALSE since this is gboolean.
> +}
>
> /* Callback function that's called when the guest sends us data */
> static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t
> len)
> @@ -48,6 +60,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const
> uint8_t *buf, size_t len)
> * do_flush_queued_data().
> */
> ret = 0;
> +qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked,
> vcon);
> }
> return ret;
> }
> --
> 1.8.1
>
>
[Qemu-devel] [PATCH 19/19] hw: compile most devices once
Most devices moved to hw/ subdirectories (other than hw/ARCH) can now be compiled once only. A notable exception is that many framebuffer devices have assumptions on the target endianness. I have no idea what would happen with OMAP in armeb mode, for example, so I'm leaving them aside. The ARM mptimer requires CPUState because of the per-CPU page, but perhaps that can be fixed since it is a target-independent field. Some bridges (lpc_ich9.c and host-typhoon.c) use interrupts from target-specific devices or CPUs, and must be compiled in the target. Perhaps they could use GPIO pins initialized in board code instead. Similarly, host-gt64xxx.c has code that should be moved in hw/mips. hw/net/milkymist-minimac2.c needs the page size. Probably it should be hardcoded to 4k? Finally, I'm leaving virtio devices as they are too. Signed-off-by: Paolo Bonzini --- hw/audio/Makefile.objs|6 +++--- hw/block/Makefile.objs|2 +- hw/char/Makefile.objs | 24 hw/display/Makefile.objs | 18 +- hw/i2c/Makefile.objs |6 +++--- hw/ide/Makefile.objs |4 ++-- hw/input/Makefile.objs|6 +++--- hw/isa/Makefile.objs |5 +++-- hw/misc/Makefile.objs |4 ++-- hw/net/Makefile.objs | 11 ++- hw/pci/Makefile.objs | 22 +- hw/scsi/Makefile.objs |2 +- hw/sd/Makefile.objs |6 +++--- hw/ssi/Makefile.objs |4 ++-- hw/timer/Makefile.objs| 27 ++- hw/usb/Makefile.objs |2 +- hw/watchdog/Makefile.objs |4 ++-- 17 files changed, 80 insertions(+), 73 deletions(-) diff --git a/hw/audio/Makefile.objs b/hw/audio/Makefile.objs index a3926f8..2375102 100644 --- a/hw/audio/Makefile.objs +++ b/hw/audio/Makefile.objs @@ -13,8 +13,8 @@ common-obj-$(CONFIG_PCSPK) += pcspk.o common-obj-$(CONFIG_WM8750) += wm8750.o common-obj-$(CONFIG_PL041) += pl041.o lm4549.o -obj-$(CONFIG_CS4231) += cs4231.o -obj-$(CONFIG_MARVELL_88W8618) += marvell_88w8618.o -obj-$(CONFIG_MILKYMIST) += milkymist-ac97.o +common-obj-$(CONFIG_CS4231) += cs4231.o +common-obj-$(CONFIG_MARVELL_88W8618) += marvell_88w8618.o +common-obj-$(CONFIG_MILKYMIST) += milkymist-ac97.o $(obj)/adlib.o $(obj)/fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0 diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs index 8b5073e..3873207 100644 --- a/hw/block/Makefile.objs +++ b/hw/block/Makefile.objs @@ -5,6 +5,6 @@ common-obj-$(CONFIG_NAND) += nand.o common-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o -obj-$(CONFIG_ONENAND) += onenand.o +common-obj-$(CONFIG_ONENAND) += onenand.o obj-$(CONFIG_VIRTIO) += dataplane/ virtio-blk.o diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs index a8850ef..7ae7dc6 100644 --- a/hw/char/Makefile.objs +++ b/hw/char/Makefile.objs @@ -8,17 +8,17 @@ common-obj-$(CONFIG_VIRTIO) += virtio-console.o common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o common-obj-$(CONFIG_CADENCE) += cadence_uart.o -obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o -obj-$(CONFIG_EXYNOS4) += exynos4210_uart.o -obj-$(CONFIG_GRLIB) += grlib_apbuart.o -obj-$(CONFIG_IMX) += imx_serial.o -obj-$(CONFIG_LM32) += lm32_juart.o -obj-$(CONFIG_LM32) += lm32_uart.o -obj-$(CONFIG_COLDFIRE) += mcf_uart.o -obj-$(CONFIG_MILKYMIST) += milkymist-uart.o -obj-$(CONFIG_OMAP) += omap_uart.o -obj-$(CONFIG_SH4) += sh_serial.o -obj-$(CONFIG_PSERIES) += spapr_vty.o -obj-$(CONFIG_SCLPCONSOLE) += sclpconsole.o +common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o +common-obj-$(CONFIG_EXYNOS4) += exynos4210_uart.o +common-obj-$(CONFIG_GRLIB) += grlib_apbuart.o +common-obj-$(CONFIG_IMX) += imx_serial.o +common-obj-$(CONFIG_LM32) += lm32_juart.o +common-obj-$(CONFIG_LM32) += lm32_uart.o +common-obj-$(CONFIG_COLDFIRE) += mcf_uart.o +common-obj-$(CONFIG_MILKYMIST) += milkymist-uart.o +common-obj-$(CONFIG_OMAP) += omap_uart.o +common-obj-$(CONFIG_SH4) += sh_serial.o +common-obj-$(CONFIG_PSERIES) += spapr_vty.o +common-obj-$(CONFIG_SCLPCONSOLE) += sclpconsole.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs index 4a8ef17..4685098 100644 --- a/hw/display/Makefile.objs +++ b/hw/display/Makefile.objs @@ -11,19 +11,19 @@ common-obj-$(CONFIG_VGA_ISA) += vga-isa.o common-obj-$(CONFIG_VGA_ISA_MM) += vga-isa-mm.o common-obj-$(CONFIG_VMWARE_VGA) += vmware_vga.o -obj-$(CONFIG_BLIZZARD) += blizzard.o -obj-$(CONFIG_EXYNOS4) += exynos4210_fimd.o -obj-$(CONFIG_FRAMEBUFFER) += framebuffer.o -obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o +common-obj-$(CONFIG_BLIZZARD) += blizzard.o +common-obj-$(CONFIG_EXYNOS4) += exynos4210_fimd.o +common-obj-$(CONFIG_FRAMEBUFFER) += framebuffer.o +common-obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o + +ifeq ($(CONFIG_OPENGL),y) +common-obj-$(CONFIG_MILKYMIST) += milkymist-tmu2.o +endif + obj-$(CONFIG_OMAP) += omap_dss.o obj-$(CONFIG_OMAP) += omap_lcdc.o obj-$(CONFIG_PXA2XX)
[Qemu-devel] [PATCH 06/19] virtio-9p: remove PCI dependencies from hw/9pfs/
Also move the 9p.h file to 9pfs/virtio-9p-device.h, for consistency
with the corresponding .c file.
Signed-off-by: Paolo Bonzini
---
hw/9pfs/virtio-9p-device.c | 53 +-
hw/{9p.h => 9pfs/virtio-9p-device.h} |4 +-
hw/9pfs/virtio-9p.c |3 +-
hw/9pfs/virtio-9p.h |1 -
hw/virtio-pci.c | 50 +++-
hw/virtio-pci.h |2 +-
hw/virtio.h |2 +-
7 files changed, 55 insertions(+), 60 deletions(-)
rename hw/{9p.h => 9pfs/virtio-9p-device.h} (85%)
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 74155fb..d321c80 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -14,9 +14,9 @@
#include "hw/virtio.h"
#include "hw/pc.h"
#include "qemu/sockets.h"
-#include "hw/virtio-pci.h"
#include "virtio-9p.h"
#include "fsdev/qemu-fsdev.h"
+#include "virtio-9p-device.h"
#include "virtio-9p-xattr.h"
#include "virtio-9p-coth.h"
@@ -136,54 +136,3 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf
*conf)
return &s->vdev;
}
-
-static int virtio_9p_init_pci(PCIDevice *pci_dev)
-{
-VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-VirtIODevice *vdev;
-
-vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf);
-vdev->nvectors = proxy->nvectors;
-virtio_init_pci(proxy, vdev);
-/* make the actual value visible */
-proxy->nvectors = vdev->nvectors;
-return 0;
-}
-
-static Property virtio_9p_properties[] = {
-DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
-DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag),
-DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id),
-DEFINE_PROP_END_OF_LIST(),
-};
-
-static void virtio_9p_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-k->init = virtio_9p_init_pci;
-k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
-k->device_id = PCI_DEVICE_ID_VIRTIO_9P;
-k->revision = VIRTIO_PCI_ABI_VERSION;
-k->class_id = 0x2;
-dc->props = virtio_9p_properties;
-dc->reset = virtio_pci_reset;
-}
-
-static const TypeInfo virtio_9p_info = {
-.name = "virtio-9p-pci",
-.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VirtIOPCIProxy),
-.class_init= virtio_9p_class_init,
-};
-
-static void virtio_9p_register_types(void)
-{
-type_register_static(&virtio_9p_info);
-virtio_9p_set_fd_limit();
-}
-
-type_init(virtio_9p_register_types)
diff --git a/hw/9p.h b/hw/9pfs/virtio-9p-device.h
similarity index 85%
rename from hw/9p.h
rename to hw/9pfs/virtio-9p-device.h
index d9951d6..65789db 100644
--- a/hw/9p.h
+++ b/hw/9pfs/virtio-9p-device.h
@@ -11,8 +11,8 @@
*
*/
-#ifndef QEMU_9P_H
-#define QEMU_9P_H
+#ifndef QEMU_VIRTIO_9P_DEVICE_H
+#define QEMU_VIRTIO_9P_DEVICE_H
typedef struct V9fsConf
{
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index b795839..a9b82a6 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -14,7 +14,6 @@
#include "hw/virtio.h"
#include "hw/pc.h"
#include "qemu/sockets.h"
-#include "hw/virtio-pci.h"
#include "virtio-9p.h"
#include "fsdev/qemu-fsdev.h"
#include "virtio-9p-xattr.h"
@@ -3267,7 +3266,7 @@ void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
free_pdu(s, pdu);
}
-void virtio_9p_set_fd_limit(void)
+static void __attribute__((__constructor__)) virtio_9p_set_fd_limit(void)
{
struct rlimit rlim;
if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 406fe52..52b1c69 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -389,7 +389,6 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
}
extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
-extern void virtio_9p_set_fd_limit(void);
extern void v9fs_reclaim_fd(V9fsPDU *pdu);
extern void v9fs_path_init(V9fsPath *path);
extern void v9fs_path_free(V9fsPath *path);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9abbcdf..6253ba5 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -255,7 +255,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
proxy->ioeventfd_started = false;
}
-void virtio_pci_reset(DeviceState *d)
+static void virtio_pci_reset(DeviceState *d)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
virtio_pci_stop_ioeventfd(proxy);
@@ -1312,6 +1312,51 @@ static const TypeInfo virtio_scsi_info = {
.class_init= virtio_scsi_class_init,
};
+#ifdef CONFIG_VIRTFS
+static int virtio_9p_init_pci(PCIDevice *pci_dev)
+{
+VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+
[Qemu-devel] [PATCH] bitops: unify bitops_flsl with host-utils.h, call it bitops_clzl
Similar to the bitops_ctz change just made.
Signed-off-by: Richard Henderson
---
include/qemu/bitops.h | 44 +++-
target-i386/topology.h | 2 +-
util/bitops.c | 2 +-
3 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 8b88791..07ada63 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -50,41 +50,27 @@ static unsigned long bitops_ctzl(unsigned long word)
}
/**
- * bitops_fls - find last (most-significant) set bit in a long word
+ * bitops_clzl - find last (most-significant) set bit in a long word
* @word: the word to search
*
* Undefined if no set bit exists, so code should check against 0 first.
*/
-static inline unsigned long bitops_flsl(unsigned long word)
+static inline unsigned long bitops_clzl(unsigned long word)
{
- int num = BITS_PER_LONG - 1;
-
-#if LONG_MAX > 0x7FFF
- if (!(word & (~0ul << 32))) {
- num -= 32;
- word <<= 32;
- }
+/* Both this function and the gcc builtin are undefined at 0, whereas
+ the host-utils functions return word-size at 0. Thus we prefer the
+ gcc builtin as the closer match. */
+#if QEMU_GNUC_PREREQ(3, 4)
+return __builtin_clzl(word);
+#else
+if (sizeof(long) == 4) {
+return clz32(word);
+} else if (sizeof(long) == 8) {
+return clz64(word);
+} else {
+abort();
+}
#endif
- if (!(word & (~0ul << (BITS_PER_LONG-16 {
- num -= 16;
- word <<= 16;
- }
- if (!(word & (~0ul << (BITS_PER_LONG-8 {
- num -= 8;
- word <<= 8;
- }
- if (!(word & (~0ul << (BITS_PER_LONG-4 {
- num -= 4;
- word <<= 4;
- }
- if (!(word & (~0ul << (BITS_PER_LONG-2 {
- num -= 2;
-
- word <<= 2;
- }
- if (!(word & (~0ul << (BITS_PER_LONG-1
- num -= 1;
- return num;
}
/**
diff --git a/target-i386/topology.h b/target-i386/topology.h
index 24ed525..96a559e 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -55,7 +55,7 @@ static unsigned apicid_bitwidth_for_count(unsigned count)
if (count == 1) {
return 0;
}
-return bitops_flsl(count - 1) + 1;
+return bitops_clzl(count - 1) + 1;
}
/* Bit width of the SMT_ID (thread ID) field on the APIC ID
diff --git a/util/bitops.c b/util/bitops.c
index 7b853cf..4f4fa95 100644
--- a/util/bitops.c
+++ b/util/bitops.c
@@ -133,7 +133,7 @@ unsigned long find_last_bit(const unsigned long *addr,
unsigned long size)
tmp = addr[--words];
if (tmp) {
found:
-return words * BITS_PER_LONG + bitops_flsl(tmp);
+return words * BITS_PER_LONG + bitops_clzl(tmp);
}
}
--
1.8.1
Re: [Qemu-devel] [PATCH 6/6] load_linux: change kernel header size allocation
On Mon, Feb 4, 2013 at 2:27 AM, liguang wrote:
> it's not necessary to alloc 8K bytes for kernel
> header, 0.5K is enough.
>
> Signed-off-by: liguang
> ---
> hw/pc.c | 10 ++
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 0ccd775..b6b236f 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -637,6 +637,9 @@ static long get_file_size(FILE *f)
> return size;
> }
>
> +#define HEADER_SIZE 0x200
> +#define HEADER_SIGNATURE 0x53726448
> +
> static void load_linux(void *fw_cfg,
> const char *kernel_filename,
> const char *initrd_filename,
> @@ -646,7 +649,7 @@ static void load_linux(void *fw_cfg,
> uint16_t protocol;
> int setup_size, kernel_size, initrd_size = 0, cmdline_size;
> uint32_t initrd_max;
> -uint8_t header[8192], *setup, *kernel, *initrd_data;
> +uint8_t header[HEADER_SIZE], *setup, *kernel, *initrd_data;
> hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> FILE *f;
> char *vmode;
> @@ -665,8 +668,7 @@ static void load_linux(void *fw_cfg,
> fprintf(stderr, "can't get size of kernel image file\n");
> exit(1);
> }
> -if (fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
> -MIN(ARRAY_SIZE(header), kernel_size)) {
> +if (fread(header, 1, HEADER_SIZE, f) <= 0) {
> fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
> kernel_filename, strerror(errno));
> exit(1);
> @@ -676,7 +678,7 @@ static void load_linux(void *fw_cfg,
> #if 0
> fprintf(stderr, "header magic: %#x\n", ldl_p(header+0x202));
> #endif
> -if (ldl_p(header+0x202) == 0x53726448)
> +if (ldl_p(header+0x202) == HEADER_SIGNATURE)
Please add braces while touching the line and add spaces around '+'.
Maybe you have not checked the patches with checkpatch.pl?
> protocol = lduw_p(header+0x206);
> else {
> /* This looks like a multiboot kernel. If it is, let's stop
> --
> 1.7.2.5
>
>
[Qemu-devel] [PATCH 4/8] vl.c: Check for NUMA node limit inside numa_add()
Instead of checking the limit before calling numa_add(), check the limit
only when we already know we're going to add a new node.
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 586aa9a..89de003 100644
--- a/vl.c
+++ b/vl.c
@@ -1258,6 +1258,12 @@ static void numa_add(const char *optarg)
optarg++;
}
if (!strcmp(option, "node")) {
+
+if (nb_numa_nodes >= MAX_NODES) {
+fprintf(stderr, "qemu: too many NUMA nodes\n");
+exit(1);
+}
+
if (get_param_value(option, 128, "nodeid", optarg) == 0) {
nodenr = nb_numa_nodes;
} else {
@@ -3003,10 +3009,6 @@ int main(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_numa:
-if (nb_numa_nodes >= MAX_NODES) {
-fprintf(stderr, "qemu: too many NUMA nodes\n");
-exit(1);
-}
numa_add(optarg);
break;
case QEMU_OPTION_display:
--
1.8.1
[Qemu-devel] [PATCH 1/8] cutils: unsigned int parsing functions
There are lots of duplicate parsing code using strto*() in QEMU, and most of that code is broken in one way or another. Even the visitors code have duplicate integer parsing code[1]. This introduces functions to help parsing unsigned int values: parse_uint() and parse_uint_full(). Parsing functions for signed ints and floats will be submitted later. parse_uint_full() has all the checks made by opts_type_uint64() at opts-visitor.c: - Check for NULL (returns -EINVAL) - Check for negative numbers (returns -EINVAL) - Check for empty string (returns -EINVAL) - Check for overflow or other errno values set by strtoll() (returns -errno) - Check for end of string (reject invalid characters after number) (returns -EINVAL) parse_uint() does everything above except checking for the end of the string, so callers can continue parsing the remainder of string after the number. Unit tests included. [1] string-input-visitor.c:parse_int() could use the same parsing code used by opts-visitor.c:opts_type_int(), instead of duplicating that logic. Signed-off-by: Eduardo Habkost Reviewed-by: Eric Blake Reviewed-by: Laszlo Ersek --- Changes on v7 are trivial: - Patch refresh after changes on test/Makefile on qemu.git master - Fix typo on comment on parse_uint() (missing whitespace) As the changes are really trivial, I am keeping the Reviewed-by lines from Eric and Laszlo. Interdiff from v6: diff -u b/tests/Makefile b/tests/Makefile --- b/tests/Makefile +++ b/tests/Makefile @@ -45,8 +45,6 @@ gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c check-unit-y += tests/test-thread-pool$(EXESUF) gcov-files-test-thread-pool-y = thread-pool.c -check-unit-y += tests/test-cutils$(EXESUF) -gcov-files-test-cutils-y += util/cutils.c check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh @@ -54,6 +52,8 @@ gcov-files-test-x86-cpuid-y = check-unit-y += tests/test-xbzrle$(EXESUF) gcov-files-test-xbzrle-y = xbzrle.c +check-unit-y += tests/test-cutils$(EXESUF) +gcov-files-test-cutils-y += util/cutils.c check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh @@ -90,7 +90,6 @@ tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(block-obj-y) libqemuutil.a libqemustub.a tests/test-iov$(EXESUF): tests/test-iov.o libqemuutil.a -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o tests/test-qapi-types.c tests/test-qapi-types.h :\ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py @@ -104,6 +103,7 @@ tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o libqemuutil.a libqemustub.a tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuutil.a +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o tests/test-qapi-types.c tests/test-qapi-types.h :\ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py diff -u b/util/cutils.c b/util/cutils.c --- b/util/cutils.c +++ b/util/cutils.c @@ -281,7 +281,7 @@ * Parse unsigned integer * * Parsed syntax is like strtoull()'s: arbitrary whitespace, a single optional - * '+' or '-', an optional "0x"if @base is 0 or 16, one or more digits. + * '+' or '-', an optional "0x" if @base is 0 or 16, one or more digits. * * If @s is null, or @base is invalid, or @s doesn't start with an * integer in the syntax above, set *@value to 0, *@endptr to @s, and --- include/qemu-common.h | 4 + tests/Makefile| 3 + tests/test-cutils.c | 251 ++ util/cutils.c | 99 4 files changed, 357 insertions(+) create mode 100644 tests/test-cutils.c diff --git a/include/qemu-common.h b/include/qemu-common.h index af2379f..80016ad 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -173,6 +173,10 @@ int qemu_fdatasync(int fd); int fcntl_setfl(int fd, int flag); int qemu_parse_fd(const char *param); +int parse_uint(const char *s, unsigned long long *value, char **endptr, + int base); +int parse_uint_full(const char *s, unsigned long long *value, int base); + /* * strtosz() suffixes used to specify the default treatment of an * argument passed to strtosz() without an explicit suffix. diff --git a/tests/Makefile b/tests/Makefile index 83145f5..a2d62b8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -52,6 +52,8 @@ check-unit-y += tests/test-x86-cpuid$(EXESUF) gcov-files-test-x86-cpuid-y = check-unit-y += tests/test-xbzrle$(EXESUF) gcov-files-test-xbzrle-y = xbzrle.c +check-unit-y += tests/test-cutils$(EXESUF) +gcov-files-test-cutils-y += util/cutils.c check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh @@ -101,6 +103,7 @@ tests/test-iov$(EXESUF): tests/te
[Qemu-devel] [PATCH 3/8] vl.c: Abort on unknown -numa option type
Abort in case an invalid -numa option is provided, instead of silently
ignoring it.
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/vl.c b/vl.c
index 0dae44c..586aa9a 100644
--- a/vl.c
+++ b/vl.c
@@ -1293,6 +1293,9 @@ static void numa_add(const char *optarg)
bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
}
nb_numa_nodes++;
+} else {
+fprintf(stderr, "Invalid -numa option: %s\n", option);
+exit(1);
}
}
--
1.8.1
[Qemu-devel] [PATCH 7/8] vl.c: Extract -numa "cpus" parsing to separate function
This will make it easier to refactor that code later.
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 41 +++--
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/vl.c b/vl.c
index d6f6422..de164f8 100644
--- a/vl.c
+++ b/vl.c
@@ -1244,15 +1244,34 @@ char *get_boot_devices_list(size_t *size)
return list;
}
+static void numa_node_parse_cpus(int nodenr, const char *cpus)
+{
+char *endptr;
+unsigned long long value, endvalue;
+
+value = strtoull(cpus, &endptr, 10);
+if (*endptr == '-') {
+endvalue = strtoull(endptr+1, &endptr, 10);
+} else {
+endvalue = value;
+}
+
+if (!(endvalue < MAX_CPUMASK_BITS)) {
+endvalue = MAX_CPUMASK_BITS - 1;
+fprintf(stderr,
+"A max of %d CPUs are supported in a guest\n",
+ MAX_CPUMASK_BITS);
+}
+
+bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+}
+
static void numa_add(const char *optarg)
{
char option[128];
char *endptr;
-unsigned long long value, endvalue;
unsigned long long nodenr;
-value = endvalue = 0ULL;
-
optarg = get_opt_name(option, 128, optarg, ',');
if (*optarg == ',') {
optarg++;
@@ -1290,21 +1309,7 @@ static void numa_add(const char *optarg)
node_mem[nodenr] = sval;
}
if (get_param_value(option, 128, "cpus", optarg) != 0) {
-value = strtoull(option, &endptr, 10);
-if (*endptr == '-') {
-endvalue = strtoull(endptr+1, &endptr, 10);
-} else {
-endvalue = value;
-}
-
-if (!(endvalue < MAX_CPUMASK_BITS)) {
-endvalue = MAX_CPUMASK_BITS - 1;
-fprintf(stderr,
-"A max of %d CPUs are supported in a guest\n",
- MAX_CPUMASK_BITS);
-}
-
-bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+numa_node_parse_cpus(nodenr, option);
}
nb_numa_nodes++;
} else {
--
1.8.1
[Qemu-devel] [PATCH 2/8] vl.c: Fix off-by-one bug when handling "-numa node" argument
The numa_add() code was unconditionally adding 1 to the get_opt_name()
return value, making it point after the end of the string if no ','
separator is present.
Example of weird behavior caused by the bug:
$ qemu-img create -f qcow2
this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2 5G
Formatting 'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2', fmt=qcow2
size=5368709120 encryption=off cluster_size=65536
$ ./x86_64-softmmu/qemu-system-x86_64 -S -monitor stdio -numa node
'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2'
QEMU 1.3.50 monitor - type 'help' for more information
(qemu) info numa
1 nodes
node 0 cpus: 0
node 0 size: 1000 MB
(qemu)
This changes the code to nove the pointer only if ',' is found.
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 3155989..0dae44c 100644
--- a/vl.c
+++ b/vl.c
@@ -1253,7 +1253,10 @@ static void numa_add(const char *optarg)
value = endvalue = 0ULL;
-optarg = get_opt_name(option, 128, optarg, ',') + 1;
+optarg = get_opt_name(option, 128, optarg, ',');
+if (*optarg == ',') {
+optarg++;
+}
if (!strcmp(option, "node")) {
if (get_param_value(option, 128, "nodeid", optarg) == 0) {
nodenr = nb_numa_nodes;
--
1.8.1
[Qemu-devel] [PATCH 5/8] vl.c: numa_add(): Validate nodeid before using it
Without this check, QEMU will corrupt memory if a too-large nodeid is
provided in the command-line. e.g.:
-numa node,mem=...,cpus=...,nodeid=65
This changes nodenr to unsigned long long, to avoid integer conversion
issues when converting the strtoull() result to int.
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
---
vl.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 89de003..4955c29 100644
--- a/vl.c
+++ b/vl.c
@@ -1249,7 +1249,7 @@ static void numa_add(const char *optarg)
char option[128];
char *endptr;
unsigned long long value, endvalue;
-int nodenr;
+unsigned long long nodenr;
value = endvalue = 0ULL;
@@ -1270,6 +1270,11 @@ static void numa_add(const char *optarg)
nodenr = strtoull(option, NULL, 10);
}
+if (nodenr >= MAX_NODES) {
+fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
+exit(1);
+}
+
if (get_param_value(option, 128, "mem", optarg) == 0) {
node_mem[nodenr] = 0;
} else {
--
1.8.1
