[Qemu-devel] [PATCH] vnc: return directly if no vnc client connected
From: ChenLiang graphic_hw_update and vnc_refresh_server_surface aren't need to do when no vnc client connected. It can reduce lock contention, because vnc_refresh will hold global big lock two millisecond every three seconds. Signed-off-by: ChenLiang Signed-off-by: Gonglei --- ui/vnc.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 0fe6eff..092ba2e 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2768,6 +2768,11 @@ static void vnc_refresh(DisplayChangeListener *dcl) VncState *vs, *vn; int has_dirty, rects = 0; +if (QTAILQ_EMPTY(&vd->clients)) { +update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_MAX); +return; +} + graphic_hw_update(NULL); if (vnc_trylock_display(vd)) { @@ -2783,11 +2788,6 @@ static void vnc_refresh(DisplayChangeListener *dcl) /* vs might be free()ed here */ } -if (QTAILQ_EMPTY(&vd->clients)) { -update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_MAX); -return; -} - if (has_dirty && rects) { vd->dcl.update_interval /= 2; if (vd->dcl.update_interval < VNC_REFRESH_INTERVAL_BASE) { -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm
Alex Bligh writes: [...] >>> +/* NB cirrus-vga default value is 8MB anyway, save if we >>> + * monkey patch it to change the default when the qemu-kvm-migration >>> + * machine parameter is selected >>> + */ >>> + >> >> This is too hacky for my taste. >> How about creating a new machine e.g. pc-qemu-kvm-1.0 and in >> pc_early_init_pci_1_0, changing compat_props for pc-1.0 to point to the >> compat_props of pc-qemu-kvm-1.0? > > Hang on a second! v2 of this patch DID use a new virtual machine, > called exactly that. I thought you were objecting to that and > wanting a machine parameter instead! It's far easier with a new > machine type, and I'd far prefer a new machine type. > > If you were just objecting to the fact that pc-1.0 was made to > be an alias of either one or the other at compile time, simply > drop the second patch of the v2 patchset. > > If we have a new machine type, I don't /think/ I need the early_init > thing at all (I may be wrong about that). I also prefer a new machine type. Ideally, the management application understands that there are two incompatibile versions QEMU (upstream and old qemu-kvm), and how to map their machine types to current QEMU's. If that's not practical, then downstream can still alias the machine types around to make things just work in the most important downstream scenarios. The most important upstream scenario is QEMU <-> QEMU, of course.
[Qemu-devel] [PATCH bugfix] snapshot: fix referencing wrong variable in while loop in do_delvm
The while loop variabal is "bs1", but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. Signed-off-by: Zhang Haoyu --- savevm.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/savevm.c b/savevm.c index e19ae0a..2d8eb96 100644 --- a/savevm.c +++ b/savevm.c @@ -1245,19 +1245,18 @@ int load_vmstate(const char *name) void do_delvm(Monitor *mon, const QDict *qdict) { -BlockDriverState *bs, *bs1; +BlockDriverState *bs; Error *err = NULL; const char *name = qdict_get_str(qdict, "name"); -bs = find_vmstate_bs(); -if (!bs) { +if (!find_vmstate_bs()) { monitor_printf(mon, "No block device supports snapshots\n"); return; } -bs1 = NULL; -while ((bs1 = bdrv_next(bs1))) { -if (bdrv_can_snapshot(bs1)) { +bs = NULL; +while ((bs = bdrv_next(bs))) { +if (bdrv_can_snapshot(bs)) { bdrv_snapshot_delete_by_id_or_name(bs, name, &err); if (err) { monitor_printf(mon, -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4 2/3] usb-hid: Add high speed mouse configuration
Hi, > Speaking about properties do I need to add something for display and > head properties? No. cheers, Gerd
[Qemu-devel] [PATCH] ssh: Don't crash if either host or path is not specified.
$ ./qemu-img create -f qcow2 overlay \ -b 'json: { "file.driver":"ssh", "file.host":"localhost", "file.host_key_check":"no" }' qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' failed. Aborted A similar crash also happens if the file.host field is omitted. https://bugzilla.redhat.com/show_bug.cgi?id=1147343 Bug found and reported by Jun Li. Signed-off-by: Richard W.M. Jones --- block/ssh.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/ssh.c b/block/ssh.c index cd2fd75..54009c0 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -517,6 +517,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, const char *host, *user, *path, *host_key_check; int port; +if (!qdict_haskey(options, "host")) { +ret = -EINVAL; +error_setg_errno(errp, errno, "No hostname was specified"); +goto err; +} host = qdict_get_str(options, "host"); if (qdict_haskey(options, "port")) { @@ -525,6 +530,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, port = 22; } +if (!qdict_haskey(options, "path")) { +ret = -EINVAL; +error_setg_errno(errp, errno, "No path was specified"); +goto err; +} path = qdict_get_str(options, "path"); if (qdict_haskey(options, "user")) { -- 2.0.4
Re: [Qemu-devel] [PATCH v6 1/2] dump: let dump_error return error info to caller
zhanghailiang writes: > The second parameter of dump_error is unused, but one purpose of > using this function is to report the error info. > > Use error_set to return the error info to the caller. > > Signed-off-by: zhanghailiang The commit message doesn't explain this patch's benefit clearly. Suggest: dump: Propagate errors into qmp_dump_guest_memory() The code calls dump_error() on error, and even passes it a suitable message. However, the message is thrown away, and its callers pass up only success/failure. All qmp_dump_guest_memory() can do is set a generic error. Propagate the errors properly, so qmp_dump_guest_memory() can return a more useful error. With a commit message like that: Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH] ssh: Don't crash if either host or path is not specified.
> Subject: [Qemu-devel] [PATCH] ssh: Don't crash if either host or path is not > specified. > > $ ./qemu-img create -f qcow2 overlay \ > -b 'json: { "file.driver":"ssh", > "file.host":"localhost", > "file.host_key_check":"no" }' > qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' > failed. > Aborted > > A similar crash also happens if the file.host field is omitted. > > https://bugzilla.redhat.com/show_bug.cgi?id=1147343 > > Bug found and reported by Jun Li. > > Signed-off-by: Richard W.M. Jones > --- > block/ssh.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/block/ssh.c b/block/ssh.c > index cd2fd75..54009c0 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -517,6 +517,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict > *options, > const char *host, *user, *path, *host_key_check; > int port; > > +if (!qdict_haskey(options, "host")) { > +ret = -EINVAL; > +error_setg_errno(errp, errno, "No hostname was specified"); I don't think errno is useful (will be a random value) here. You just need use error_setg(), and return ret directly. :) > +goto err; > +} > host = qdict_get_str(options, "host"); > > if (qdict_haskey(options, "port")) { > @@ -525,6 +530,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict > *options, > port = 22; > } > > +if (!qdict_haskey(options, "path")) { > +ret = -EINVAL; > +error_setg_errno(errp, errno, "No path was specified"); Here, too. > +goto err; > +} > path = qdict_get_str(options, "path"); > > if (qdict_haskey(options, "user")) { > -- > 2.0.4 > BTW, you'd better CC corresponding maintainer in the next version. You can get the maintainer by "./script/get_maintainer.pl" script. Best regards, -Gonglei
[Qemu-devel] [PATCH v2] ssh: Don't crash if either host or path is not specified.
$ ./qemu-img create -f qcow2 overlay \ -b 'json: { "file.driver":"ssh", "file.host":"localhost", "file.host_key_check":"no" }' qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' failed. Aborted A similar crash also happens if the file.host field is omitted. https://bugzilla.redhat.com/show_bug.cgi?id=1147343 Bug found and reported by Jun Li. Signed-off-by: Richard W.M. Jones --- block/ssh.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/ssh.c b/block/ssh.c index cd2fd75..35f143d 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -517,6 +517,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, const char *host, *user, *path, *host_key_check; int port; +if (!qdict_haskey(options, "host")) { +ret = -EINVAL; +error_setg(errp, "No hostname was specified"); +goto err; +} host = qdict_get_str(options, "host"); if (qdict_haskey(options, "port")) { @@ -525,6 +530,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, port = 22; } +if (!qdict_haskey(options, "path")) { +ret = -EINVAL; +error_setg(errp, "No path was specified"); +goto err; +} path = qdict_get_str(options, "path"); if (qdict_haskey(options, "user")) { -- 2.0.4
Re: [Qemu-devel] [PATCH v6 1/2] dump: let dump_error return error info to caller
On 2014/9/29 15:48, Markus Armbruster wrote: zhanghailiang writes: The second parameter of dump_error is unused, but one purpose of using this function is to report the error info. Use error_set to return the error info to the caller. Signed-off-by: zhanghailiang The commit message doesn't explain this patch's benefit clearly. Suggest: dump: Propagate errors into qmp_dump_guest_memory() The code calls dump_error() on error, and even passes it a suitable message. However, the message is thrown away, and its callers pass up only success/failure. All qmp_dump_guest_memory() can do is set a generic error. Propagate the errors properly, so qmp_dump_guest_memory() can return a more useful error. With a commit message like that: Reviewed-by: Markus Armbruster OK, Will fix like that, and submit V7, thanks very much;)
Re: [Qemu-devel] [PATCH v6 2/2] dump: Don't return error code when return an Error object
zhanghailiang writes: > Functions shouldn't return an error code and an Error object at the same time. > Turn all these functions that returning Error object to void. > We also judge if a function success or fail by reference to the local_err. > > Signed-off-by: zhanghailiang > > --- > dump.c | 313 > +++-- > 1 file changed, 148 insertions(+), 165 deletions(-) > > diff --git a/dump.c b/dump.c > index 07d2300..a6188b3 100644 > --- a/dump.c > +++ b/dump.c > @@ -100,7 +100,7 @@ static int fd_write_vmcore(const void *buf, size_t size, > void *opaque) > return 0; > } > > -static int write_elf64_header(DumpState *s, Error **errp) > +static void write_elf64_header(DumpState *s, Error **errp) > { > Elf64_Ehdr elf_header; > int ret; > @@ -128,13 +128,10 @@ static int write_elf64_header(DumpState *s, Error > **errp) > ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s); > if (ret < 0) { > dump_error(s, "dump: failed to write elf header", errp); > -return -1; > } > - > -return 0; > } > > -static int write_elf32_header(DumpState *s, Error **errp) > +static void write_elf32_header(DumpState *s, Error **errp) > { > Elf32_Ehdr elf_header; > int ret; > @@ -162,13 +159,10 @@ static int write_elf32_header(DumpState *s, Error > **errp) > ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s); > if (ret < 0) { > dump_error(s, "dump: failed to write elf header", errp); > -return -1; > } > - > -return 0; > } > > -static int write_elf64_load(DumpState *s, MemoryMapping *memory_mapping, > +static void write_elf64_load(DumpState *s, MemoryMapping *memory_mapping, > int phdr_index, hwaddr offset, > hwaddr filesz, Error **errp) > { > @@ -188,15 +182,12 @@ static int write_elf64_load(DumpState *s, MemoryMapping > *memory_mapping, > ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s); > if (ret < 0) { > dump_error(s, "dump: failed to write program header table", errp); > -return -1; > } > - > -return 0; > } > > -static int write_elf32_load(DumpState *s, MemoryMapping *memory_mapping, > -int phdr_index, hwaddr offset, > -hwaddr filesz, Error **errp) > +static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping, > + int phdr_index, hwaddr offset, > + hwaddr filesz, Error **errp) > { > Elf32_Phdr phdr; > int ret; > @@ -214,13 +205,10 @@ static int write_elf32_load(DumpState *s, MemoryMapping > *memory_mapping, > ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s); > if (ret < 0) { > dump_error(s, "dump: failed to write program header table", errp); > -return -1; > } > - > -return 0; > } > > -static int write_elf64_note(DumpState *s, Error **errp) > +static void write_elf64_note(DumpState *s, Error **errp) > { > Elf64_Phdr phdr; > hwaddr begin = s->memory_offset - s->note_size; > @@ -237,10 +225,7 @@ static int write_elf64_note(DumpState *s, Error **errp) > ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s); > if (ret < 0) { > dump_error(s, "dump: failed to write program header table", errp); > -return -1; > } > - > -return 0; > } > > static inline int cpu_index(CPUState *cpu) > @@ -248,8 +233,8 @@ static inline int cpu_index(CPUState *cpu) > return cpu->cpu_index + 1; > } > > -static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s, > - Error **errp) > +static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s, > + Error **errp) > { > CPUState *cpu; > int ret; > @@ -260,7 +245,7 @@ static int write_elf64_notes(WriteCoreDumpFunction f, > DumpState *s, > ret = cpu_write_elf64_note(f, cpu, id, s); > if (ret < 0) { > dump_error(s, "dump: failed to write elf notes", errp); > -return -1; > +return; > } > } > > @@ -268,14 +253,12 @@ static int write_elf64_notes(WriteCoreDumpFunction f, > DumpState *s, > ret = cpu_write_elf64_qemunote(f, cpu, s); > if (ret < 0) { > dump_error(s, "dump: failed to write CPU status", errp); > -return -1; > +return; > } > } > - > -return 0; > } > > -static int write_elf32_note(DumpState *s, Error **errp) > +static void write_elf32_note(DumpState *s, Error **errp) > { > hwaddr begin = s->memory_offset - s->note_size; > Elf32_Phdr phdr; > @@ -292,14 +275,11 @@ static int write_elf32_note(DumpState *s, Error **errp) > ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s); > if (ret < 0) { > dump_error(s, "dump: failed to write program header tab
Re: [Qemu-devel] [PATCH] vnc: return directly if no vnc client connected
On Mo, 2014-09-29 at 15:00 +0800, arei.gong...@huawei.com wrote: > graphic_hw_update and vnc_refresh_server_surface aren't > need to do when no vnc client connected. It can reduce > lock contention, because vnc_refresh will hold global big > lock two millisecond every three seconds. Added to vnc patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
On 26.09.14 20:47, Don Slutz wrote: > This adds synchronisation of the vcpu registers > between Xen and QEMU. > > Signed-off-by: Don Slutz Could you instead plug into the existing cpu_synchronize_registers() framework and just implement register synchronization for the Xen side, just like it's been done for KVM? :) Alex
Re: [Qemu-devel] [PATCH bugfix] snapshot: fix referencing wrong variable in while loop in do_delvm
"Zhang Haoyu" writes: > The while loop variabal is "bs1", but "bs" is always passed to > bdrv_snapshot_delete_by_id_or_name. Please limit commit message line length to around 70 characters. > > Signed-off-by: Zhang Haoyu > --- > savevm.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/savevm.c b/savevm.c > index e19ae0a..2d8eb96 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -1245,19 +1245,18 @@ int load_vmstate(const char *name) > > void do_delvm(Monitor *mon, const QDict *qdict) > { > -BlockDriverState *bs, *bs1; > +BlockDriverState *bs; > Error *err = NULL; > const char *name = qdict_get_str(qdict, "name"); > > -bs = find_vmstate_bs(); > -if (!bs) { > +if (!find_vmstate_bs()) { > monitor_printf(mon, "No block device supports snapshots\n"); > return; > } > > -bs1 = NULL; > -while ((bs1 = bdrv_next(bs1))) { > -if (bdrv_can_snapshot(bs1)) { > +bs = NULL; > +while ((bs = bdrv_next(bs))) { > +if (bdrv_can_snapshot(bs)) { > bdrv_snapshot_delete_by_id_or_name(bs, name, &err); > if (err) { > monitor_printf(mon, Ouch! Broken in commit a89d89d, v1.7.0. If you add that to your commit message, you can also add Reviewed-by: Markus Armbruster
Re: [Qemu-devel] Ping [PATCH] gtk: add support for the Pause key
On So, 2014-09-28 at 16:54 +0200, Martin Decky wrote: > http://patchwork.ozlabs.org/patch/390074/ > > It is a rather simple patch adding support for the Pause key in the GTK > UI. It should not cause any regressions. Please apply. Thanks. Patch looks good but doesn't apply any more. Can you please rebase & resend? thanks, Gerd
Re: [Qemu-devel] [PATCH v3 5/7] sysbus: Add new platform bus helper device
On 26.09.14 14:44, Paolo Bonzini wrote: > Il 26/09/2014 14:26, Alexander Graf ha scritto: >> >> Are you sure? Imagine one sysbus device includes another. We only want >> to look at the region the lowest sysbus device exposes, no? > > IIUC this function is used to build the device tree. Yes, it's used to figure out the map of "start of region x of my device" to "offset y in the platform bus mmio space". > Say you have 2 > consecutive memory regions and the device tree requires separate "reg" > entries for them. But because they are consecutive (or perhaps because > you have a PCI version of the same device that sticks them in a single > BAR) you use a single MMIO area at the sysbus level. > > In that case, you will use platform_bus_get_mmio_addr on the two inner > regions, not the outer one. In that case, you will use platform_bus_get_mmio_addr on the outer region because that's what the device model exposes. The parameter to this function that tells us which region we want is the "mmio region number" that sysbus exposes. If in device tree there are 2 reg properties, the device tree assembling code has to do the conversion from sysbus granularity to device tree granularity :). > > BTW, I think you will never have one sysbus device including another. > The contained device would be busless (similar to the "naked" 8250 > device in hw/char/serial.c, except perhaps QOMified). Yeah, I agree :). Alex
Re: [Qemu-devel] [PATCH v4 13/19] qapi: More rigourous checking of types
Markus Armbruster writes: > Eric Blake writes: > >> Now that we know every expression is valid with regards to >> its keys, we can add further tests that those keys refer to >> valid types. With this patch, all references to a type (the >> 'data': of command, type, union, and event, and the 'returns': >> of command) must resolve to a builtin or another type declared >> by the current qapi parse; this includes recursing into each >> member of a data dictionary. Dealing with '**' and nested >> sub-structs will be done in later patches. >> >> Update the testsuite to match improved output. >> >> Signed-off-by: Eric Blake [...] >> @@ -262,6 +308,15 @@ def check_command(expr, expr_info): >> raise QAPIExprError(expr_info, >> "command '%s' is already defined" % name) >> commands.append(name) >> +check_type(expr_info, "'data' for command '%s'" % name, >> + expr.get('data'), allow_array=True, >> + allowed_names=['union', 'struct']) >> +check_type(expr_info, "'base' for command '%s'" % name, >> + expr.get('base'), allowed_names=['struct'], >> + allow_dict=False) >> +check_type(expr_info, "'returns' for command '%s'" % name, >> + expr.get('returns'), allow_array=True, >> + allowed_names=['built-in', 'union', 'struct', 'enum']) >> > > Nicely done. Wait a sec! What's a command's 'base'? [...]
Re: [Qemu-devel] [PATCH v2] ssh: Don't crash if either host or path is not specified.
> -Original Message- > From: Richard W.M. Jones [mailto:rjo...@redhat.com] > Sent: Monday, September 29, 2014 4:06 PM > To: qemu-devel@nongnu.org > Cc: Gonglei (Arei) > Subject: [PATCH v2] ssh: Don't crash if either host or path is not specified. > > $ ./qemu-img create -f qcow2 overlay \ > -b 'json: { "file.driver":"ssh", > "file.host":"localhost", > "file.host_key_check":"no" }' > qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' > failed. > Aborted > > A similar crash also happens if the file.host field is omitted. > > https://bugzilla.redhat.com/show_bug.cgi?id=1147343 > > Bug found and reported by Jun Li. > > Signed-off-by: Richard W.M. Jones > --- > block/ssh.c | 10 ++ > 1 file changed, 10 insertions(+) > Cc'ing Stefan and Kevin. You can add the change log between v1 and v2. Anyway, looks good to me, so Reviewed-by: Gonglei Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v3 5/7] sysbus: Add new platform bus helper device
Il 29/09/2014 10:24, Alexander Graf ha scritto: > > > On 26.09.14 14:44, Paolo Bonzini wrote: >> Il 26/09/2014 14:26, Alexander Graf ha scritto: >>> >>> Are you sure? Imagine one sysbus device includes another. We only want >>> to look at the region the lowest sysbus device exposes, no? >> >> IIUC this function is used to build the device tree. > > Yes, it's used to figure out the map of "start of region x of my device" > to "offset y in the platform bus mmio space". > >> Say you have 2 >> consecutive memory regions and the device tree requires separate "reg" >> entries for them. But because they are consecutive (or perhaps because >> you have a PCI version of the same device that sticks them in a single >> BAR) you use a single MMIO area at the sysbus level. >> >> In that case, you will use platform_bus_get_mmio_addr on the two inner >> regions, not the outer one. > > In that case, you will use platform_bus_get_mmio_addr on the outer > region because that's what the device model exposes. The parameter to > this function that tells us which region we want is the "mmio region > number" that sysbus exposes. > > If in device tree there are 2 reg properties, the device tree assembling > code has to do the conversion from sysbus granularity to device tree > granularity :). Thanks for the clarification. Series Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration
On 26.09.14 04:53, Alexey Kardashevskiy wrote: > On 09/26/2014 12:31 PM, David Gibson wrote: >> On Thu, Sep 25, 2014 at 08:06:40PM +1000, Alexey Kardashevskiy wrote: >>> On 09/25/2014 07:43 PM, Alexander Graf wrote: On 25.09.14 09:02, Alexey Kardashevskiy wrote: > The only case when sPAPR NVRAM migrates now is if is backed by a file and > copy-storage migration is performed. > > This enables RAM copy of NVRAM even if NVRAM is backed by a file. > > This defines a VMSTATE descriptor for NVRAM device so the memory copy > of NVRAM can migrate and be written to a backing file on the destination > if one is provided. > > Signed-off-by: Alexey Kardashevskiy > --- > hw/nvram/spapr_nvram.c | 68 > +++--- > 1 file changed, 59 insertions(+), 9 deletions(-) > > diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c > index 6a72ef4..254009e 100644 > --- a/hw/nvram/spapr_nvram.c > +++ b/hw/nvram/spapr_nvram.c > @@ -76,15 +76,20 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, > sPAPREnvironment *spapr, > return; > } > > +assert(nvram->buf); > + > membuf = cpu_physical_memory_map(buffer, &len, 1); > + > +alen = len; > if (nvram->drive) { > alen = bdrv_pread(nvram->drive, offset, membuf, len); > +if (alen > 0) { > +memcpy(nvram->buf + offset, membuf, alen); Why? >>> >>> This way I do not need pre_save hook and I keep the buf in sync with the >>> file. If I implement pre_save, then buf will serve 2 purposes - it is >>> either NVRAM itself (if there is no backing file, exists during guest's >>> lifetime) or it is a migration copy (exists between pre_save and post_load >>> and then it is disposed). Two quite different uses of the same thing >>> confuse me. But - I do not mind doing it your way, no big deal, >>> should I? >> >> This doesn't seem quite right to me. I don't see anything that pulls >> in the whole of the nvram contents at initialization, so it looks like >> the buffer will only be in sync with the driver for the portions that >> are either read or written by the guest. Then, if you migrate while >> not all of the memory copy is in sync, you could clobber the >> out-of-sync parts of the disk copy as well. > > Yes. I missed that :-/ > > >> Instead, I think you need to suck in the whole of the contents during >> init, then all reads can just be supplied from the memory buffer, and >> you'll only need to access the backing disk for stores. > > I like this and I will do this if Alex does not mind. So you'd always keep a shadow copy in RAM and only use the file for writes? Sounds like a good plan to me. Alex
Re: [Qemu-devel] [PATCH v4 14/19] qapi: More rigorous checking for type safety bypass
Eric Blake writes: > Now that we have a way to validate every type, we can also be > stricter about enforcing that callers that want to bypass > type safety in generated code. Prior to this patch, it didn't > matter what value was associated with the key 'gen', but it > looked odd that 'gen':'yes' could result in bypassing the > generated code. These changes also enforce the changes made > earlier in the series for documentation and consolidation of > using '**' as the wildcard type. Perhaps it's worth mentioning that the schema has always used 'gen': 'no'. That said, 'no' is silly. The natural JSON for a flag is false / true! Since you're touching it anyway, consider making it an optional boolean defaulting to false. Same for the other silly 'no': success-response. > Signed-off-by: Eric Blake > --- > scripts/qapi.py| 21 - > tests/qapi-schema/type-bypass-bad-gen.err | 1 + > tests/qapi-schema/type-bypass-bad-gen.exit | 2 +- > tests/qapi-schema/type-bypass-bad-gen.json | 2 +- > tests/qapi-schema/type-bypass-bad-gen.out | 3 --- > tests/qapi-schema/type-bypass-no-gen.err | 1 + > tests/qapi-schema/type-bypass-no-gen.exit | 2 +- > tests/qapi-schema/type-bypass-no-gen.json | 2 +- > tests/qapi-schema/type-bypass-no-gen.out | 3 --- > 9 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 20c0ce9..15972c6 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -256,13 +256,13 @@ def discriminator_find_enum_define(expr): > return find_enum(discriminator_type) > > def check_type(expr_info, source, data, allow_array = False, > - allowed_names = [], allow_dict = True): > + allowed_names = [], allow_dict = True, allow_star = False): > global all_types > > if data is None: > return > > -if data == '**': > +if allow_star and data == '**': > return > > # Check if array type for data is okay > @@ -278,6 +278,10 @@ def check_type(expr_info, source, data, allow_array = > False, > > # Check if type name for data is okay > if isinstance(data, basestring): > +if data == '**': > +raise QAPIExprError(expr_info, > +"%s uses '**' but did not request 'gen':'no'" > +% source) > if not data in all_types: > raise QAPIExprError(expr_info, > "%s references unknown type '%s'" I'm blind: I can't see where this error gets suppressed when we have 'gen': 'no'. > @@ -299,24 +303,27 @@ def check_type(expr_info, source, data, allow_array = > False, > check_type(expr_info, "member '%s' of %s" % (key, source), value, > allow_array=True, > allowed_names=['built-in', 'union', 'struct', 'enum'], > - allow_dict=True) > + allow_dict=True, allow_star=allow_star) > allow_star applies recursively. Correct, because... > def check_command(expr, expr_info): > global commands > name = expr['command'] > +allow_star = expr.has_key('gen') > + > if name in commands: > raise QAPIExprError(expr_info, > "command '%s' is already defined" % name) > commands.append(name) > check_type(expr_info, "'data' for command '%s'" % name, > expr.get('data'), allow_array=True, > - allowed_names=['union', 'struct']) > + allowed_names=['union', 'struct'], allow_star=allow_star) > check_type(expr_info, "'base' for command '%s'" % name, > expr.get('base'), allowed_names=['struct'], > allow_dict=False) > check_type(expr_info, "'returns' for command '%s'" % name, > expr.get('returns'), allow_array=True, > - allowed_names=['built-in', 'union', 'struct', 'enum']) > + allowed_names=['built-in', 'union', 'struct', 'enum'], > + allow_star=allow_star) > ... it applies exactly to a command's 'data' and 'returns' when it has 'gen': 'no'. Good. > def check_event(expr, expr_info): > global events > @@ -450,6 +457,10 @@ def check_keys(expr_elem, meta, required, optional=[]): > raise QAPIExprError(info, > "%s '%s' has unknown key '%s'" > % (meta, name, key)) > +if (key == 'gen' or key == 'success-response') and value != 'no': > +raise QAPIExprError(info, > +"'%s' of %s '%s' should only have value 'no'" > +% (key, meta, name)) > for key in required: > if not expr.has_key(key): > raise QAPIExprError(info, [...]
[Qemu-devel] [PATCH bugfix v2] snapshot: fix referencing wrong variable in while loop in do_delvm
The while loop variabal is "bs1", but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. Broken in commit a89d89d, v1.7.0. v1 -> v2: * add broken commit id to commit message Signed-off-by: Zhang Haoyu Reviewed-by: Markus Armbruster --- savevm.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/savevm.c b/savevm.c index e19ae0a..2d8eb96 100644 --- a/savevm.c +++ b/savevm.c @@ -1245,19 +1245,18 @@ int load_vmstate(const char *name) void do_delvm(Monitor *mon, const QDict *qdict) { -BlockDriverState *bs, *bs1; +BlockDriverState *bs; Error *err = NULL; const char *name = qdict_get_str(qdict, "name"); -bs = find_vmstate_bs(); -if (!bs) { +if (!find_vmstate_bs()) { monitor_printf(mon, "No block device supports snapshots\n"); return; } -bs1 = NULL; -while ((bs1 = bdrv_next(bs1))) { -if (bdrv_can_snapshot(bs1)) { +bs = NULL; +while ((bs = bdrv_next(bs))) { +if (bdrv_can_snapshot(bs)) { bdrv_snapshot_delete_by_id_or_name(bs, name, &err); if (err) { monitor_printf(mon, -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4 1/2] QEMUSizedBuffer based QEMUFile
* Eric Blake (ebl...@redhat.com) wrote: > On 09/17/2014 05:26 AM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > > > This is based on Stefan and Joel's patch that creates a QEMUFile that goes > > to a memory buffer; from: > > > > http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05036.html > > Sheesh - a year and a half ago, and still not ready to merge. Yes; I'm dragging it screaming, hopefully over the line. > > Using the QEMUFile interface, this patch adds support functions for > > operating on in-memory sized buffers that can be written to or read from. > > > > Signed-off-by: Stefan Berger > > Signed-off-by: Joel Schopp > > > > For fixes/tweeks I've done: > > Signed-off-by: Dr. David Alan Gilbert > > --- > > include/migration/qemu-file.h | 28 +++ > > include/qemu/typedefs.h | 1 + > > qemu-file.c | 457 > > ++ > > 3 files changed, 486 insertions(+) > > > > > +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len) > > > +for (i = 0; i < num_chunks; i++) { > > +qsb->iov[i].iov_base = g_try_malloc0(chunk_size); > > +if (!qsb->iov[i].iov_base) { > > +size_t j; > > + > > +for (j = 0; j < i; j++) { > > +g_free(qsb->iov[j].iov_base); > > +} > > +g_free(qsb->iov); > > +g_free(qsb); > > +return NULL; > > Rather than inlining all this cleanup, you could just call > qsb_free(qsb). But I can live with this version too. Done; I was wary at first, but I checked and since qsb_free uses g_free, and g_free is defined to ignore NULL pointers, it's OK to be used at this point. > > +/** > > + * Grow the QEMUSizedBuffer to the given size and allocated > > + * memory for it. > > s/allocated/allocate/ ? Fixed. > > > + * > > + * @qsb: A QEMUSizedBuffer > > + * @new_size: The new size of the buffer > > + * > > + * Returns an error code in case of memory allocation failure > > s/an error code/a negative error code/ ? Done; reformatted that text to lay the 'or' out more clearly. > > + * or the new size of the buffer otherwise. The returned size > > + * may be greater or equal to @new_size. > > + */ > > +static ssize_t qsb_grow(QEMUSizedBuffer *qsb, size_t new_size) > > +{ > > +size_t needed_chunks, i; > > + > > +if (qsb->size < new_size) { > > +struct iovec *new_iov; > > +size_t size_diff = new_size - qsb->size; > > +size_t chunk_size = (size_diff > QSB_MAX_CHUNK_SIZE) > > + ? QSB_MAX_CHUNK_SIZE : QSB_CHUNK_SIZE; > > + > > +needed_chunks = DIV_ROUND_UP(size_diff, chunk_size); > > + > > +new_iov = g_try_malloc_n(qsb->n_iov + needed_chunks, > > + sizeof(struct iovec)); > > Indentation is off. Done. > > + > > +const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f) > > +{ > > +QEMUBuffer *p; > > + > > +qemu_fflush(f); > > + > > +p = (QEMUBuffer *)f->opaque; > > Cast is not necessary (this is C, after all, not C++). Done. > > + > > +return p->qsb; > > +} > > + > > +static const QEMUFileOps buf_read_ops = { > > +.get_buffer = buf_get_buffer, > > +.close = buf_close > > +}; > > I think we prefer trailing commas, if only so that future additions > don't have to modify existing lines. > > > + > > +static const QEMUFileOps buf_write_ops = { > > +.put_buffer = buf_put_buffer, > > +.close = buf_close > > +}; > > and again. Done. > > > + > > +QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input) > > +{ > > +QEMUBuffer *s; > > + > > +if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != > > 0) { > > I prefer '\0' over 0 when comparing characters. Or shorthand such as > '|| *mode[1]'. But it works as is. Done as '\0' > > +error_report("qemu_bufopen: Argument validity check failed"); > > +return NULL; > > +} > > + > > +s = g_malloc0(sizeof(QEMUBuffer)); > > +if (mode[0] == 'r') { > > +s->qsb = input; > > +} > > + > > +if (s->qsb == NULL) { > > +s->qsb = qsb_create(NULL, 0); > > +} > > +if (!s->qsb) { > > +error_report("qemu_bufopen: qsb_create failed"); > > +return NULL; > > Memory leak of s. Fixed. > > +} > > + > > + > > +if (mode[0] == 'r') { > > +s->file = qemu_fopen_ops(s, &buf_read_ops); > > +} else { > > +s->file = qemu_fopen_ops(s, &buf_write_ops); > > +} > > +return s->file; > > +} > > > > Closer; most of my findings are minor, but the memleak needs fixing. If > the only changes you make are in relation to my findings, then v5 can > start life with Reviewed-by: Eric Blake Thanks! I'll get that posted shortly, so please just check it over. Dave > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Dr. David Alan Gilbert / dg
Re: [Qemu-devel] [RFC PATCH v0 10/15] ppc: Factor out CPU initialization code to a new routine
On Mon, 29 Sep 2014 08:30:15 +0530 Bharata B Rao wrote: > On Fri, Sep 26, 2014 at 05:29:02PM +0200, Igor Mammedov wrote: > > On Thu, 4 Sep 2014 11:36:20 +0530 > > Bharata B Rao wrote: > > > > > Separate out CPU initialization code into a new routine > > > ppc_new_cpu() so that it can be used from CPU hotplug path too. > > > > > > Signed-off-by: Bharata B Rao > > > --- > > > hw/ppc/spapr.c | 73 > > > +- 1 file > > > changed, 42 insertions(+), 31 deletions(-) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index b2ca527..41207ae 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -1603,6 +1603,45 @@ static SaveVMHandlers savevm_htab_handlers > > > = { .load_state = htab_load, > > > }; > > > > > looking at following code from POV of using CPU with device_add cmd. > > > > > +static const char *current_cpu_model; > > do PPC CPUs use full 'model-name,feature1,-feature2' format or only > > model-name from cpu_model string? > > I think it is just the model-name. Then without lagacy baggage that x86 has, it should be possible to convert PPC to device_add/del supported object. You wont't need a global to keep model name, it will come as type name device_add command. > > > > > > +static PowerPCCPU *ppc_new_cpu(const char *cpu_model) > > > +{ > > > +PowerPCCPU *cpu; > > > +CPUPPCState *env; > > > + > > > +cpu = cpu_ppc_init(cpu_model); for fully QOMified CPU you don't need cpu_ppc_init() object_new() should be sufficient. > > > +if (cpu == NULL) { > > > +fprintf(stderr, "Unable to find PowerPC CPU > > > definition\n"); > > > +exit(1); > > > +} > > > > -- cut -- > > > +env = &cpu->env; > > > + > > > +/* Set time-base frequency to 512 MHz */ > > > +cpu_ppc_tb_init(env, TIMEBASE_FREQ); > > > + > > > +/* PAPR always has exception vectors in RAM not ROM. To > > > ensure this, > > > + * MSR[IP] should never be set. > > > + */ > > > +env->msr_mask &= ~(1 << 6); > > > + > > > +/* Tell KVM that we're in PAPR mode */ > > > +if (kvm_enabled()) { > > > +kvmppc_set_papr(cpu); > > > +} > > > + > > > +if (cpu->max_compat) { > > > +if (ppc_set_compat(cpu, cpu->max_compat) < 0) { > > > +exit(1); > > > +} > > > +} > > -- cut -- > > selected block looks like setting CPU internals, which could be done > > inside of CPU's realizefn. > > > > > + > > > +xics_cpu_setup(spapr->icp, cpu); xics_cpu_setup() looks like a wiring of CPU with external component. I'd move it into plug handler and handle it in PPCMAchine object. It should be possible to split/move the rest of this function into initfn()/realize() of CPU. > > > > > > > +qemu_register_reset(spapr_cpu_reset, cpu); > > also could be put inside of CPU's realizefn, like it's done in > > for x86 CPU. > > Right, I just converted my CPU hotplug patchset for PowerPC to use the > hotplug handler APIs and now working on to see if I can switch over to > device_add and support device_del for CPU hotplug on PowerPC. > > Regards, > Bharata. >
Re: [Qemu-devel] [PATCH v3] pc-dimm/numa: Fix stat of memory size in node when hotplug memory
On 2014/9/26 19:53, Igor Mammedov wrote: On Tue, 23 Sep 2014 16:11:25 +0800 zhanghailiang wrote: When do memory hotplug, if there is numa node, we should add the memory size to the corresponding node memory size. For now, it mainly affects the result of hmp command "info numa". Signed-off-by: zhanghailiang please make sure that this doesn't breaks other targets. PS: to make test builds you can use travis-ci.org+github service OK, Will do this. Thanks. --- v3: - cold-plugged memory should not be excluded when stat memory size (Igor Mammedov) v2: - Don't modify the numa_info.node_mem directly when treating hotplug memory, fix the "info numa" instead (suggested by Igor Mammedov) --- hw/mem/pc-dimm.c | 30 ++ include/hw/mem/pc-dimm.h | 2 ++ include/sysemu/sysemu.h | 1 + monitor.c| 6 +- numa.c | 15 +++ 5 files changed, 53 insertions(+), 1 deletion(-) diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 5bfc5b7..8e80d74 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -195,6 +195,36 @@ out: return ret; } +static int pc_dimm_stat_mem_size(Object *obj, void *opaque) +{ +uint64_t *node_mem = opaque; +int ret; + +if (object_dynamic_cast(obj, TYPE_PC_DIMM)) { +DeviceState *dev = DEVICE(obj); + +if (dev->realized) { +PCDIMMDevice *dimm = PC_DIMM(obj); +int size; + +size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, + NULL); +if (size < 0) { +return -1; +} +node_mem[dimm->node] += size; +} +} + +ret = object_child_foreach(obj, pc_dimm_stat_mem_size, opaque); +return ret; +} + +void pc_dimm_stat_node_mem(uint64_t *node_mem) +{ +object_child_foreach(qdev_get_machine(), pc_dimm_stat_mem_size, node_mem); +} + static Property pc_dimm_properties[] = { DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0), DEFINE_PROP_UINT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, 0), diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index 761eeef..0c9a8eb 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -78,4 +78,6 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp); int qmp_pc_dimm_device_list(Object *obj, void *opaque); + +void pc_dimm_stat_node_mem(uint64_t *node_mem); #endif diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index d8539fd..cfc1592 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -160,6 +160,7 @@ typedef struct node_info { extern NodeInfo numa_info[MAX_NODES]; void set_numa_nodes(void); void set_numa_modes(void); +int query_numa_node_mem(uint64_t *node_mem); extern QemuOptsList qemu_numa_opts; int numa_init_func(QemuOpts *opts, void *opaque); diff --git a/monitor.c b/monitor.c index 7467521..c8c812f 100644 --- a/monitor.c +++ b/monitor.c @@ -1948,7 +1948,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict) { int i; CPUState *cpu; +uint64_t *node_mem; +node_mem = g_new0(uint64_t, nb_numa_nodes); +query_numa_node_mem(node_mem); monitor_printf(mon, "%d nodes\n", nb_numa_nodes); for (i = 0; i < nb_numa_nodes; i++) { monitor_printf(mon, "node %d cpus:", i); @@ -1959,8 +1962,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict) } monitor_printf(mon, "\n"); monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i, -numa_info[i].node_mem >> 20); +node_mem[i] >> 20); } +g_free(node_mem); } #ifdef CONFIG_PROFILER diff --git a/numa.c b/numa.c index 3b98135..4e27dd8 100644 --- a/numa.c +++ b/numa.c @@ -35,6 +35,7 @@ #include "hw/boards.h" #include "sysemu/hostmem.h" #include "qmp-commands.h" +#include "hw/mem/pc-dimm.h" QemuOptsList qemu_numa_opts = { .name = "numa", @@ -315,6 +316,20 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, } } +int query_numa_node_mem(uint64_t *node_mem) +{ +int i; + +if (nb_numa_nodes <= 0) { +return 0; +} +pc_dimm_stat_node_mem(node_mem); +for (i = 0; i < nb_numa_nodes; i++) { +node_mem[i] += numa_info[i].node_mem; +} +return 0; +} + static int query_memdev(Object *obj, void *opaque) { MemdevList **list = opaque; .
Re: [Qemu-devel] [PATCH V3 4/7] pc: add cpu hotplug handler to PC_MACHINE
On Mon, 29 Sep 2014 10:58:04 +0800 Gu Zheng wrote: > Hi Igor, > On 09/26/2014 09:06 PM, Igor Mammedov wrote: > > > On Wed, 17 Sep 2014 09:24:00 +0800 > > Gu Zheng wrote: > > > >> Add cpu hotplug handler to PC_MACHINE, which will perform the acpi > >> cpu hotplug callback via hotplug_handler API. > >> > >> v3: > >> -deal with start up cpus in a more neat way as Igor suggested. > >> v2: > >> -just rebase. > >> > >> Signed-off-by: Gu Zheng > >> --- > >> hw/i386/pc.c | 26 +- > >> 1 files changed, 25 insertions(+), 1 deletions(-) > >> > >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c > >> index b6c9b61..e25e71b 100644 > >> --- a/hw/i386/pc.c > >> +++ b/hw/i386/pc.c > >> @@ -1613,11 +1613,34 @@ out: > >> error_propagate(errp, local_err); > >> } > >> > >> +static void pc_cpu_plug(HotplugHandler *hotplug_dev, > >> +DeviceState *dev, Error **errp) > >> +{ > >> +HotplugHandlerClass *hhc; > >> +Error *local_err = NULL; > >> +PCMachineState *pcms = PC_MACHINE(hotplug_dev); > >> + > >> +if (!pcms->acpi_dev) { > >> +if (dev->hotplugged) { > >> +error_setg(&local_err, > >> + "cpu hotplug is not enabled: missing acpi > >> device"); > >> +} > >> +goto out; > >> +} > >> + > >> +hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); > >> +hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); > > above call this would rise SCI unconditionally whether CPU is > > hotplugged or not. > > Above precheck can avoid this, startup CPUs won't run into this: > if (!pcms->acpi_dev) { > if (dev->hotplugged) { > error_setg(&local_err, >"cpu hotplug is not enabled: missing acpi > device"); } > goto out; > } that's true, but it robs plug handler of initializing startup CPUs' state in acpi_dev and leads to code duplication between AcpiCpuHotplug_init() and acpi_cpu_plug_cb(). > > Thanks, > Gu > > > perhaps checking for this in acpi_cpu_plug_cb() and rising IRQ only > > for hotplugged CPUs would be better. > > > >> +out: > >> +error_propagate(errp, local_err); > >> +} > >> + > >> static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, > >>DeviceState *dev, Error > >> **errp) { > >> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > >> pc_dimm_plug(hotplug_dev, dev, errp); > >> +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > >> +pc_cpu_plug(hotplug_dev, dev, errp); > >> } > >> } > >> > >> @@ -1626,7 +1649,8 @@ static HotplugHandler > >> *pc_get_hotpug_handler(MachineState *machine, { > >> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine); > >> > >> -if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > >> +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || > >> +object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > >> return HOTPLUG_HANDLER(machine); > >> } > >> > > > > . > > > >
Re: [Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
Hi, > > /me wonders what happened to the input-send-event patch from marcelo, > > see http://patchwork.ozlabs.org/patch/360649/ > > > > According to patchwork I've picked it up. But it is neither upstream > > nor in my local input branch. And I can't remember what happened :( > > Marcelo, any clue? Maybe I should just re-queue it ... > > I was wondering the same... just requeue please. Let me know if it fails > to apply and i'll rebase/resend. Doesn't apply any more, so yes, please rebase & resend. thanks, Gerd
Re: [Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
Hi, > It doesn't matter, so users might release the modifier key or not. > we should make both works > > 1) > sendkey Ctrl-Scroll > sendkey Ctrl-Scroll Good to know this works. > 2) > sendkey Ctrl-Scroll-Scroll Why? This tries to squeeze something into the sendkey interface which it doesn't was designed for, and IMO this isn't a good idea, especially if we have something better at hand (marcelos patch). cheers, Gerd
Re: [Qemu-devel] [PATCH bugfix v2] snapshot: fix referencing wrong variable in while loop in do_delvm
Dropping cc: xiaw...@linux.ibm.com, because I got a "user unknown" bounce. Copying qemu-stable. "Zhang Haoyu" writes: > The while loop variabal is "bs1", > but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. > Broken in commit a89d89d, v1.7.0. > > v1 -> v2: > * add broken commit id to commit message Patch version information... > Signed-off-by: Zhang Haoyu > Reviewed-by: Markus Armbruster > --- ... goes here, so it doesn't go into the permanent commit message. Maybe Kevin or Stefan can clean it up on commit. > savevm.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) [...]
Re: [Qemu-devel] [PATCH] main-loop: Use epoll on Linux
On Mon, 09/29 13:26, Fam Zheng wrote: > A new implementation for qemu_poll_ns based on epoll is introduced here > to address the slowness of g_poll and ppoll when the number of fds are > high. > > On my laptop this would reduce the virtio-blk on top of null-aio > device's response time from 32 us to 29 us with few fds (~10), and 48 us > to 32 us with more fds (for example when virtio-serial is plugged and > ~64 more io handlers are enabled). > > Signed-off-by: Fam Zheng > --- > Makefile.objs| 1 + > include/qemu/main-loop.h | 1 + > qemu-epoll.c | 165 > +++ > qemu-timer.c | 4 +- > tests/Makefile | 2 +- > 5 files changed, 171 insertions(+), 2 deletions(-) > create mode 100644 qemu-epoll.c > > diff --git a/Makefile.objs b/Makefile.objs > index 97db978..52ee086 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -9,6 +9,7 @@ util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o > qapi-event.o > block-obj-y = async.o thread-pool.o > block-obj-y += nbd.o block.o blockjob.o > block-obj-y += main-loop.o iohandler.o qemu-timer.o > +block-obj-$(CONFIG_LINUX) += qemu-epoll.o > block-obj-$(CONFIG_POSIX) += aio-posix.o > block-obj-$(CONFIG_WIN32) += aio-win32.o > block-obj-y += block/ > diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h > index 62c68c0..eb01b95 100644 > --- a/include/qemu/main-loop.h > +++ b/include/qemu/main-loop.h > @@ -307,5 +307,6 @@ void qemu_iohandler_poll(GArray *pollfds, int rc); > > QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque); > void qemu_bh_schedule_idle(QEMUBH *bh); > +int qemu_epoll(GPollFD *fds, guint nfds, int64_t timeout); > > #endif > diff --git a/qemu-epoll.c b/qemu-epoll.c > new file mode 100644 > index 000..89ec12a > --- /dev/null > +++ b/qemu-epoll.c > @@ -0,0 +1,165 @@ > +/* > + * QEMU Event Loop > + * > + * Copyright (c) 2014 Red Hat, Inc. > + * > + * Authors: > + * Fam Zheng > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > deal > + * in the Software without restriction, including without limitation the > rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#include > +#include "qemu/main-loop.h" > + > +static bool g_poll_fds_changed(const GPollFD *fds_a, const guint nfds_a, > + const GPollFD *fds_b, const guint nfds_b) > +{ > +int i; > + > +if (nfds_a != nfds_b) { > +return true; > +} > +if (!!fds_a != !!fds_b) { > +return true; > +} > +for (i = 0; i < nfds_a; i++) { > +if (fds_a[i].fd != fds_b[i].fd || > +fds_a[i].events != fds_b[i].events) { > +return true; > +} > +} > +return false; > +} > + > +static inline int g_io_condition_from_epoll_events(int e) > +{ > +return (e & EPOLLIN ? G_IO_IN : 0) | > + (e & EPOLLOUT ? G_IO_OUT : 0) | > + (e & EPOLLERR ? G_IO_ERR : 0) | > + (e & EPOLLHUP ? G_IO_HUP : 0); > +} > + > +static inline void epoll_event_from_g_poll_fd(struct epoll_event *event, > + GPollFD *fd) > +{ > +int e = fd->events; > + > +event->events = (e & G_IO_IN ? EPOLLIN : 0) | > +(e & G_IO_OUT ? EPOLLOUT : 0) | > +(e & G_IO_ERR ? EPOLLERR : 0) | > +(e & G_IO_HUP ? EPOLLHUP : 0); > +event->data.ptr = fd; > +} > + > +static int epoll_prepare(int epollfd, > + GPollFD *fds, guint nfds, > + GPollFD **g_poll_fds, > + guint *g_poll_nfds, > + int **g_poll_fd_idx) > +{ > +int i; > + > +GPollFD *pfds = NULL; > +int npfds = 0; > +int *idx = NULL; > + > +for (i = 0; i < nfds; i++) { > +int r; > +struct epoll_event event; > +epoll_event_from_g_poll_fd(&event, &fds[i]); > + > +r = epoll_ctl(epollfd, EPOLL_CTL_ADD, fds[i].fd, &event); > +if (r) { > +
Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API
Hi, > Series unifies different hotplug mechanisms to a recent > hotplug-handler API and does shallow conversion of > devices that still use legacy qdev hotplug to it dropping > not used after that legacy hotplug path [29/30]. > It also relaces SCSI's own way to do hotplug/unplug with > hotplug-handler callbacks leaving it the only method > perform hotplug tasks. > And the last patch [30/30] allows to unplug of BUS-less > devices using hotplug-handler API. USB bits look good to me now. cheers, Gerd
[Qemu-devel] [PATCH v2 00/11] vga: cleanup and endianness patch series
Hi, Here comes v2 of the vga patches, this time the two patch series combined into one. Fixed issues found by David Gibson in review, fixed some codestyle issues. Nothing major. Should be ready for merge now, /me plans to send a pull req later this week, possibly excluding patch #11 as I'd like to have ppc64le testing feedback for that one first. please review, Gerd Benjamin Herrenschmidt (10): vga: Start cutting out non-32bpp conversion support vga: Remove remainder of old conversion cruft vga: Separate LE and BE conversion functions vga: Remove rgb_to_pixel indirection vga: Simplify vga_draw_blank() a bit cirrus: Remove non-32bpp cursor drawing vga: Remove some "should be done in BIOS" comments vga: Rename vga_template.h to vga-helpers.h vga: Make fb endian a common state variable vga: Add endian control register Gerd Hoffmann (1): vga-pci: add qext region to mmio docs/specs/standard-vga.txt | 9 + hw/display/cirrus_vga.c | 79 +++--- hw/display/cirrus_vga_template.h | 102 --- hw/display/{vga_template.h => vga-helpers.h} | 318 +++--- hw/display/vga-pci.c | 68 + hw/display/vga.c | 382 +++ hw/display/vga_int.h | 5 +- include/hw/i386/pc.h | 8 + 8 files changed, 381 insertions(+), 590 deletions(-) delete mode 100644 hw/display/cirrus_vga_template.h rename hw/display/{vga_template.h => vga-helpers.h} (52%) -- 1.8.3.1
[Qemu-devel] [PATCH v2 07/11] vga: Remove some "should be done in BIOS" comments
From: Benjamin Herrenschmidt Not all platforms have a VGA BIOS, powerpc typically relies on using the DISPI interface to initialize the card. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index c3df0c5..21aa2c6 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -761,14 +761,13 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) s->vbe_regs[VBE_DISPI_INDEX_ENABLE] |= VBE_DISPI_ENABLED; vbe_fixup_regs(s); -/* clear the screen (should be done in BIOS) */ +/* clear the screen */ if (!(val & VBE_DISPI_NOCLEARMEM)) { memset(s->vram_ptr, 0, s->vbe_regs[VBE_DISPI_INDEX_YRES] * s->vbe_line_offset); } -/* we initialize the VGA graphic mode (should be done - in BIOS) */ +/* we initialize the VGA graphic mode */ /* graphic mode + memory map 1 */ s->gr[VGA_GFX_MISC] = (s->gr[VGA_GFX_MISC] & ~0x0c) | 0x04 | VGA_GR06_GRAPHICS_MODE; @@ -801,7 +800,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) (shift_control << 5); s->cr[VGA_CRTC_MAX_SCAN] &= ~0x9f; /* no double scan */ } else { -/* XXX: the bios should do that */ s->bank_offset = 0; } s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0; -- 1.8.3.1
[Qemu-devel] [PATCH v2 01/11] vga: Start cutting out non-32bpp conversion support
From: Benjamin Herrenschmidt Nowadays, we either share a surface with the host, or we create a 32bpp ARGB console surface. So we only need to draw/convert to 32bpp, enabling us to remove all but one instance of vga_template.h inclusion (to be further cleaned up), rgb_to_pixel_* etc... Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 258 +-- 1 file changed, 22 insertions(+), 236 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index df0c010..47e5d70 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1006,81 +1006,12 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) } } -typedef void vga_draw_glyph8_func(uint8_t *d, int linesize, - const uint8_t *font_ptr, int h, - uint32_t fgcol, uint32_t bgcol); -typedef void vga_draw_glyph9_func(uint8_t *d, int linesize, - const uint8_t *font_ptr, int h, - uint32_t fgcol, uint32_t bgcol, int dup9); typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#define DEPTH 8 -#include "vga_template.h" - -#define DEPTH 15 -#include "vga_template.h" - -#define BGR_FORMAT -#define DEPTH 15 -#include "vga_template.h" - -#define DEPTH 16 -#include "vga_template.h" - -#define BGR_FORMAT -#define DEPTH 16 -#include "vga_template.h" - #define DEPTH 32 #include "vga_template.h" -#define BGR_FORMAT -#define DEPTH 32 -#include "vga_template.h" - -static unsigned int rgb_to_pixel8_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel8(r, g, b); -col |= col << 8; -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel15_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel15(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel15bgr_dup(unsigned int r, unsigned int g, - unsigned int b) -{ -unsigned int col; -col = rgb_to_pixel15bgr(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel16_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel16(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel16bgr_dup(unsigned int r, unsigned int g, - unsigned int b) -{ -unsigned int col; -col = rgb_to_pixel16bgr(r, g, b); -col |= col << 16; -return col; -} static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) { @@ -1089,13 +1020,6 @@ static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned return col; } -static unsigned int rgb_to_pixel32bgr_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel32bgr(r, g, b); -return col; -} - /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) { @@ -1202,56 +1126,6 @@ static int update_basic_params(VGACommonState *s) return full_update; } -#define NB_DEPTHS 7 - -static inline int get_depth_index(DisplaySurface *s) -{ -switch (surface_bits_per_pixel(s)) { -default: -case 8: -return 0; -case 15: -return 1; -case 16: -return 2; -case 32: -if (is_surface_bgr(s)) { -return 4; -} else { -return 3; -} -} -} - -static vga_draw_glyph8_func * const vga_draw_glyph8_table[NB_DEPTHS] = { -vga_draw_glyph8_8, -vga_draw_glyph8_16, -vga_draw_glyph8_16, -vga_draw_glyph8_32, -vga_draw_glyph8_32, -vga_draw_glyph8_16, -vga_draw_glyph8_16, -}; - -static vga_draw_glyph8_func * const vga_draw_glyph16_table[NB_DEPTHS] = { -vga_draw_glyph16_8, -vga_draw_glyph16_16, -vga_draw_glyph16_16, -vga_draw_glyph16_32, -vga_draw_glyph16_32, -vga_draw_glyph16_16, -vga_draw_glyph16_16, -}; - -static vga_draw_glyph9_func * const vga_draw_glyph9_table[NB_DEPTHS] = { -vga_draw_glyph9_8, -vga_draw_glyph9_16, -vga_draw_glyph9_16, -vga_draw_glyph9_32, -vga_draw_glyph9_32, -vga_draw_glyph9_16, -vga_draw_glyph9_16, -}; static const uint8_t cursor_glyph[32 * 4] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, @@ -1303,18 +1177,6 @@ static void vga_get_text_resolution(VGACommonState *s, int *pwidth, int *pheight *pcheight = cheight; } -typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b); - -static rgb_to_pixel_dup_func * const rgb_to_pixel_dup_table[NB_DEPTHS] = { -rgb_to_pixel8_dup, -rgb_to_pixel15_dup, -rgb_to_pixel16_dup, -rgb_to_pixel32_dup, -rgb_to_pixel32bgr
[Qemu-devel] [PATCH v2 09/11] vga: Make fb endian a common state variable
From: Benjamin Herrenschmidt And initialize it based on target endian Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 32 +++- hw/display/vga_int.h | 1 + 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 9f60f88..49a4b8b 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1461,16 +1461,11 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) int disp_width, multi_scan, multi_run; uint8_t *d; uint32_t v, addr1, addr; -vga_draw_line_func *vga_draw_line; -#if defined(TARGET_WORDS_BIGENDIAN) -static const bool big_endian_fb = true; -#else -static const bool big_endian_fb = false; -#endif -#if defined(HOST_WORDS_BIGENDIAN) -static const bool byteswap = !big_endian_fb; +vga_draw_line_func *vga_draw_line = NULL; +#ifdef HOST_WORDS_BIGENDIAN +bool byteswap = !s->big_endian_fb; #else -static const bool byteswap = big_endian_fb; +bool byteswap = s->big_endian_fb; #endif full_update |= update_basic_params(s); @@ -1573,19 +1568,19 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) bits = 8; break; case 15: -v = big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; bits = 16; break; case 16: -v = big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; bits = 16; break; case 24: -v = big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; bits = 24; break; case 32: -v = big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; bits = 32; break; } @@ -2129,6 +2124,17 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) s->update_retrace_info = vga_precise_update_retrace_info; break; } + +/* + * Set default fb endian based on target, should probably be turned + * into a device attribute set by the machine/platform to remove + * all target endian dependencies from this file. + */ +#ifdef TARGET_WORDS_BIGENDIAN +s->big_endian_fb = true; +#else +s->big_endian_fb = false; +#endif vga_dirty_log_start(s); } diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 9073370..28d67cf 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -157,6 +157,7 @@ typedef struct VGACommonState { const GraphicHwOps *hw_ops; bool full_update_text; bool full_update_gfx; +bool big_endian_fb; /* hardware mouse cursor support */ uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; void (*cursor_invalidate)(struct VGACommonState *s); -- 1.8.3.1
[Qemu-devel] [PATCH v2 11/11] vga-pci: add qext region to mmio
Add a qemu extented register range to the standard vga mmio bar. Right nowe there are two registers: One readonly register returning the size of the region (so we can easily add more registers there if needed) and one endian control register, so guests (especially ppc) can flip the framebuffer endianness as they need it. Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- docs/specs/standard-vga.txt | 9 ++ hw/display/vga-pci.c| 68 + include/hw/i386/pc.h| 8 ++ 3 files changed, 85 insertions(+) diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt index f82773e..19d2a74 100644 --- a/docs/specs/standard-vga.txt +++ b/docs/specs/standard-vga.txt @@ -70,3 +70,12 @@ Likewise applies to the pci variant only for obvious reasons. 0500 - 0515 : bochs dispi interface registers, mapped flat without index/data ports. Use (index << 1) as offset for (16bit) register access. + +0600 - 0607 : qemu extended registers. qemu 2.2+ only. + The pci revision is 2 (or greater) when + these registers are present. The registers + are 32bit. + 0600 : qemu extended register region size, in bytes. + 0604 : framebuffer endianness register. + - 0xbebebebe indicates big endian. + - 0x1e1e1e1e indicates little endian. diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index 0351d94..3394ec2 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -35,10 +35,18 @@ #define PCI_VGA_IOPORT_SIZE (0x3e0 - 0x3c0) #define PCI_VGA_BOCHS_OFFSET 0x500 #define PCI_VGA_BOCHS_SIZE(0x0b * 2) +#define PCI_VGA_QEXT_OFFSET 0x600 +#define PCI_VGA_QEXT_SIZE (2 * 4) #define PCI_VGA_MMIO_SIZE 0x1000 +#define PCI_VGA_QEXT_REG_SIZE (0 * 4) +#define PCI_VGA_QEXT_REG_BYTEORDER(1 * 4) +#define PCI_VGA_QEXT_LITTLE_ENDIAN 0x1e1e1e1e +#define PCI_VGA_QEXT_BIG_ENDIAN 0xbebebebe + enum vga_pci_flags { PCI_VGA_FLAG_ENABLE_MMIO = 1, +PCI_VGA_FLAG_ENABLE_QEXT = 2, }; typedef struct PCIVGAState { @@ -48,6 +56,7 @@ typedef struct PCIVGAState { MemoryRegion mmio; MemoryRegion ioport; MemoryRegion bochs; +MemoryRegion qext; } PCIVGAState; static const VMStateDescription vmstate_vga_pci = { @@ -140,6 +149,46 @@ static const MemoryRegionOps pci_vga_bochs_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +static uint64_t pci_vga_qext_read(void *ptr, hwaddr addr, unsigned size) +{ +PCIVGAState *d = ptr; + +switch (addr) { +case PCI_VGA_QEXT_REG_SIZE: +return PCI_VGA_QEXT_SIZE; +case PCI_VGA_QEXT_REG_BYTEORDER: +return d->vga.big_endian_fb ? +PCI_VGA_QEXT_BIG_ENDIAN : PCI_VGA_QEXT_LITTLE_ENDIAN; +default: +return 0; +} +} + +static void pci_vga_qext_write(void *ptr, hwaddr addr, + uint64_t val, unsigned size) +{ +PCIVGAState *d = ptr; + +switch (addr) { +case PCI_VGA_QEXT_REG_BYTEORDER: +if (val == PCI_VGA_QEXT_BIG_ENDIAN) { +d->vga.big_endian_fb = true; +} +if (val == PCI_VGA_QEXT_LITTLE_ENDIAN) { +d->vga.big_endian_fb = false; +} +break; +} +} + +static const MemoryRegionOps pci_vga_qext_ops = { +.read = pci_vga_qext_read, +.write = pci_vga_qext_write, +.valid.min_access_size = 4, +.valid.max_access_size = 4, +.endianness = DEVICE_LITTLE_ENDIAN, +}; + static int pci_std_vga_initfn(PCIDevice *dev) { PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev); @@ -167,6 +216,15 @@ static int pci_std_vga_initfn(PCIDevice *dev) &d->ioport); memory_region_add_subregion(&d->mmio, PCI_VGA_BOCHS_OFFSET, &d->bochs); + +if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { +memory_region_init_io(&d->qext, NULL, &pci_vga_qext_ops, d, + "qemu extended regs", PCI_VGA_QEXT_SIZE); +memory_region_add_subregion(&d->mmio, PCI_VGA_QEXT_OFFSET, +&d->qext); +pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2); +} + pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); } @@ -199,6 +257,14 @@ static int pci_secondary_vga_initfn(PCIDevice *dev) memory_region_add_subregion(&d->mmio, PCI_VGA_BOCHS_OFFSET, &d->bochs); +if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { +memory_region_init_io(&d->qext, NULL, &pci_vga_qext_ops, d, + "qemu extended regs", PCI_VGA_QEXT_SIZE); +memory_region_add_subregion(&d->mmio, PCI_VGA_QEXT_OFFSET, +&d->qext); +pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2); +} + pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PR
[Qemu-devel] [PATCH v2 08/11] vga: Rename vga_template.h to vga-helpers.h
From: Benjamin Herrenschmidt It's no longer a template, we only instanciate the file once. Keep it a #included file so the functions remain static. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/{vga_template.h => vga-helpers.h} | 0 hw/display/vga.c | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename hw/display/{vga_template.h => vga-helpers.h} (100%) diff --git a/hw/display/vga_template.h b/hw/display/vga-helpers.h similarity index 100% rename from hw/display/vga_template.h rename to hw/display/vga-helpers.h diff --git a/hw/display/vga.c b/hw/display/vga.c index 21aa2c6..9f60f88 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1007,7 +1007,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#include "vga_template.h" +#include "vga-helpers.h" /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) -- 1.8.3.1
[Qemu-devel] [PATCH v2 06/11] cirrus: Remove non-32bpp cursor drawing
From: Benjamin Herrenschmidt We only draw cursor on non-shared surfaces (so it seems...) and these are always 32bpp Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/cirrus_vga.c | 64 +--- hw/display/cirrus_vga_template.h | 102 --- 2 files changed, 36 insertions(+), 130 deletions(-) delete mode 100644 hw/display/cirrus_vga_template.h diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 6f8d149..8a5b76c 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2171,20 +2171,44 @@ static void cirrus_cursor_invalidate(VGACommonState *s1) } } -#define DEPTH 8 -#include "cirrus_vga_template.h" - -#define DEPTH 16 -#include "cirrus_vga_template.h" - -#define DEPTH 32 -#include "cirrus_vga_template.h" +static void vga_draw_cursor_line(uint8_t *d1, + const uint8_t *src1, + int poffset, int w, + unsigned int color0, + unsigned int color1, + unsigned int color_xor) +{ +const uint8_t *plane0, *plane1; +int x, b0, b1; +uint8_t *d; + +d = d1; +plane0 = src1; +plane1 = src1 + poffset; +for (x = 0; x < w; x++) { +b0 = (plane0[x >> 3] >> (7 - (x & 7))) & 1; +b1 = (plane1[x >> 3] >> (7 - (x & 7))) & 1; +switch (b0 | (b1 << 1)) { +case 0: +break; +case 1: +((uint32_t *)d)[0] ^= color_xor; +break; +case 2: +((uint32_t *)d)[0] = color0; +break; +case 3: +((uint32_t *)d)[0] = color1; +break; +} +d += 4; +} +} static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) { CirrusVGAState *s = container_of(s1, CirrusVGAState, vga); -DisplaySurface *surface = qemu_console_surface(s->vga.con); -int w, h, bpp, x1, x2, poffset; +int w, h, x1, x2, poffset; unsigned int color0, color1; const uint8_t *palette, *src; uint32_t content; @@ -2238,24 +2262,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]), c6_to_8(palette[0xf * 3 + 1]), c6_to_8(palette[0xf * 3 + 2])); -bpp = surface_bytes_per_pixel(surface); -d1 += x1 * bpp; -switch (surface_bits_per_pixel(surface)) { -default: -break; -case 8: -vga_draw_cursor_line_8(d1, src, poffset, w, color0, color1, 0xff); -break; -case 15: -vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x7fff); -break; -case 16: -vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x); -break; -case 32: -vga_draw_cursor_line_32(d1, src, poffset, w, color0, color1, 0xff); -break; -} +d1 += x1 * 4; +vga_draw_cursor_line(d1, src, poffset, w, color0, color1, 0xff); } /*** diff --git a/hw/display/cirrus_vga_template.h b/hw/display/cirrus_vga_template.h deleted file mode 100644 index 3b28280..000 --- a/hw/display/cirrus_vga_template.h +++ /dev/null @@ -1,102 +0,0 @@ -/* - * QEMU Cirrus VGA Emulator templates - * - * Copyright (c) 2003 Fabrice Bellard - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -#if DEPTH == 8 -#define BPP 1 -#elif DEPTH == 15 || DEPTH == 16 -#define BPP 2 -#elif DEPTH == 32 -#define BPP 4 -#else -#error unsupported depth -#endif - -static void glue(vga_draw_cursor_line_, DEPTH)(uint8_t *d1, - const uint8_t *src1, - int poffset, int w, -
[Qemu-devel] [PATCH v2 05/11] vga: Simplify vga_draw_blank() a bit
From: Benjamin Herrenschmidt The test for surface_bits_per_pixel() isn't necessary anymore, the 8bpp case never happens. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 9d06691..c3df0c5 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1682,7 +1682,7 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) static void vga_draw_blank(VGACommonState *s, int full_update) { DisplaySurface *surface = qemu_console_surface(s->con); -int i, w, val; +int i, w; uint8_t *d; if (!full_update) @@ -1690,15 +1690,10 @@ static void vga_draw_blank(VGACommonState *s, int full_update) if (s->last_scr_width <= 0 || s->last_scr_height <= 0) return; -if (surface_bits_per_pixel(surface) == 8) { -val = rgb_to_pixel32(0, 0, 0); -} else { -val = 0; -} w = s->last_scr_width * surface_bytes_per_pixel(surface); d = surface_data(surface); for(i = 0; i < s->last_scr_height; i++) { -memset(d, val, w); +memset(d, 0, w); d += surface_stride(surface); } dpy_gfx_update(s->con, 0, 0, -- 1.8.3.1
[Qemu-devel] [PATCH v2 04/11] vga: Remove rgb_to_pixel indirection
From: Benjamin Herrenschmidt We always use rgb_to_pixel32 nowadays. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/cirrus_vga.c | 15 +-- hw/display/vga.c| 31 ++- hw/display/vga_int.h| 2 -- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index db330e9..6f8d149 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -29,6 +29,7 @@ #include "hw/hw.h" #include "hw/pci/pci.h" #include "ui/console.h" +#include "ui/pixel_ops.h" #include "vga_int.h" #include "hw/loader.h" @@ -2212,6 +2213,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) } else { src += (s->vga.sr[0x13] & 0x3f) * 256; src += (scr_y - s->hw_cursor_y) * 4; + + poffset = 128; content = ((uint32_t *)src)[0] | ((uint32_t *)(src + 128))[0]; @@ -2229,12 +2232,12 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) x2 = s->vga.last_scr_width; w = x2 - x1; palette = s->cirrus_hidden_palette; -color0 = s->vga.rgb_to_pixel(c6_to_8(palette[0x0 * 3]), - c6_to_8(palette[0x0 * 3 + 1]), - c6_to_8(palette[0x0 * 3 + 2])); -color1 = s->vga.rgb_to_pixel(c6_to_8(palette[0xf * 3]), - c6_to_8(palette[0xf * 3 + 1]), - c6_to_8(palette[0xf * 3 + 2])); +color0 = rgb_to_pixel32(c6_to_8(palette[0x0 * 3]), +c6_to_8(palette[0x0 * 3 + 1]), +c6_to_8(palette[0x0 * 3 + 2])); +color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]), +c6_to_8(palette[0xf * 3 + 1]), +c6_to_8(palette[0xf * 3 + 2])); bpp = surface_bytes_per_pixel(surface); d1 += x1 * bpp; switch (surface_bits_per_pixel(surface)) { diff --git a/hw/display/vga.c b/hw/display/vga.c index 451c354..9d06691 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1011,13 +1011,6 @@ typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, #include "vga_template.h" -static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel32(r, g, b); -return col; -} - /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) { @@ -1034,9 +1027,9 @@ static int update_palette16(VGACommonState *s) v = ((s->ar[VGA_ATC_COLOR_PAGE] & 0xc) << 4) | (v & 0x3f); } v = v * 3; -col = s->rgb_to_pixel(c6_to_8(s->palette[v]), - c6_to_8(s->palette[v + 1]), - c6_to_8(s->palette[v + 2])); +col = rgb_to_pixel32(c6_to_8(s->palette[v]), + c6_to_8(s->palette[v + 1]), + c6_to_8(s->palette[v + 2])); if (col != palette[i]) { full_update = 1; palette[i] = col; @@ -1056,13 +1049,13 @@ static int update_palette256(VGACommonState *s) v = 0; for(i = 0; i < 256; i++) { if (s->dac_8bit) { - col = s->rgb_to_pixel(s->palette[v], -s->palette[v + 1], -s->palette[v + 2]); +col = rgb_to_pixel32(s->palette[v], + s->palette[v + 1], + s->palette[v + 2]); } else { - col = s->rgb_to_pixel(c6_to_8(s->palette[v]), -c6_to_8(s->palette[v + 1]), -c6_to_8(s->palette[v + 2])); +col = rgb_to_pixel32(c6_to_8(s->palette[v]), + c6_to_8(s->palette[v + 1]), + c6_to_8(s->palette[v + 2])); } if (col != palette[i]) { full_update = 1; @@ -1245,7 +1238,6 @@ static void vga_draw_text(VGACommonState *s, int full_update) s->last_cw = cw; full_update = 1; } -s->rgb_to_pixel = rgb_to_pixel32_dup; full_update |= update_palette16(s); palette = s->last_palette; x_incr = cw * surface_bytes_per_pixel(surface); @@ -1553,8 +1545,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) dpy_gfx_replace_surface(s->con, surface); } -s->rgb_to_pixel = rgb_to_pixel32_dup; - if (shift_control == 0) { full_update |= update_palette16(s); if (s->sr[VGA_SEQ_CLOCK_MODE] & 8) { @@ -1700,9 +1690,8 @@ static void vga_draw_blank(VGACommonState *s, int full_update) if (s->last_scr_width <= 0 || s->last_scr_height <= 0) return; -s->rgb_to_pixel = rgb_to_pixel32_dup; if (surface_bits_per_pixel(surface) == 8) { -val = s->rgb_to
[Qemu-devel] [PATCH v2 02/11] vga: Remove remainder of old conversion cruft
From: Benjamin Herrenschmidt All the macros used to generate different versions of vga_template.h are now unnecessary, take them all out and remove the _32 suffix from most functions. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 46 +- hw/display/vga_template.h | 227 +++--- 2 files changed, 95 insertions(+), 178 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 47e5d70..721309f 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1009,10 +1009,8 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#define DEPTH 32 #include "vga_template.h" - static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) { unsigned int col; @@ -1311,19 +1309,19 @@ static void vga_draw_text(VGACommonState *s, int full_update) bgcol = palette[cattr >> 4]; fgcol = palette[cattr & 0x0f]; if (cw == 16) { -vga_draw_glyph16_32(d1, linesize, -font_ptr, cheight, fgcol, bgcol); +vga_draw_glyph16(d1, linesize, + font_ptr, cheight, fgcol, bgcol); } else if (cw != 9) { -vga_draw_glyph8_32(d1, linesize, - font_ptr, cheight, fgcol, bgcol); +vga_draw_glyph8(d1, linesize, +font_ptr, cheight, fgcol, bgcol); } else { dup9 = 0; if (ch >= 0xb0 && ch <= 0xdf && (s->ar[VGA_ATC_MODE] & 0x04)) { dup9 = 1; } -vga_draw_glyph9_32(d1, linesize, - font_ptr, cheight, fgcol, bgcol, dup9); +vga_draw_glyph9(d1, linesize, +font_ptr, cheight, fgcol, bgcol, dup9); } if (src == cursor_ptr && !(s->cr[VGA_CRTC_CURSOR_START] & 0x20) && @@ -1339,14 +1337,14 @@ static void vga_draw_text(VGACommonState *s, int full_update) h = line_last - line_start + 1; d = d1 + linesize * line_start; if (cw == 16) { -vga_draw_glyph16_32(d, linesize, - cursor_glyph, h, fgcol, bgcol); +vga_draw_glyph16(d, linesize, + cursor_glyph, h, fgcol, bgcol); } else if (cw != 9) { -vga_draw_glyph8_32(d, linesize, - cursor_glyph, h, fgcol, bgcol); +vga_draw_glyph8(d, linesize, +cursor_glyph, h, fgcol, bgcol); } else { -vga_draw_glyph9_32(d, linesize, - cursor_glyph, h, fgcol, bgcol, 1); +vga_draw_glyph9(d, linesize, +cursor_glyph, h, fgcol, bgcol, 1); } } } @@ -1384,16 +1382,16 @@ enum { }; static vga_draw_line_func * const vga_draw_line_table[VGA_DRAW_LINE_NB] = { -vga_draw_line2_32, -vga_draw_line2d2_32, -vga_draw_line4_32, -vga_draw_line4d2_32, -vga_draw_line8d2_32, -vga_draw_line8_32, -vga_draw_line15_32, -vga_draw_line16_32, -vga_draw_line24_32, -vga_draw_line32_32, +vga_draw_line2, +vga_draw_line2d2, +vga_draw_line4, +vga_draw_line4d2, +vga_draw_line8d2, +vga_draw_line8, +vga_draw_line15, +vga_draw_line16, +vga_draw_line24, +vga_draw_line32, }; static int vga_get_bpp(VGACommonState *s) diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h index 90ec9c2..0660b52 100644 --- a/hw/display/vga_template.h +++ b/hw/display/vga_template.h @@ -22,41 +22,9 @@ * THE SOFTWARE. */ -#if DEPTH == 8 -#define BPP 1 -#define PIXEL_TYPE uint8_t -#elif DEPTH == 15 || DEPTH == 16 -#define BPP 2 -#define PIXEL_TYPE uint16_t -#elif DEPTH == 32 -#define BPP 4 -#define PIXEL_TYPE uint32_t -#else -#error unsupport depth -#endif - -#ifdef BGR_FORMAT -#define PIXEL_NAME glue(DEPTH, bgr) -#else -#define PIXEL_NAME DEPTH -#endif /* BGR_FORMAT */ - -#if DEPTH != 15 && !defined(BGR_FORMAT) - -static inline void glue(vga_draw_glyph_line_, DEPTH)(uint8_t *d, - uint32_t font_data, - u
[Qemu-devel] [PATCH v2 03/11] vga: Separate LE and BE conversion functions
From: Benjamin Herrenschmidt Provide different functions for converting from an LE vs a BE framebuffer. We cannot rely on the simple cases always being shared surfaces since cirrus will need to always shadow for cursor emulation, so we need the full set of functions to be able to later handle runtime switching. Signed-off-by: Benjamin Herrenschmidt \ Signed-off-by: Gerd Hoffmann --- hw/display/vga.c | 43 +++--- hw/display/vga_template.h | 109 +++--- 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 721309f..451c354 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1374,10 +1374,14 @@ enum { VGA_DRAW_LINE4D2, VGA_DRAW_LINE8D2, VGA_DRAW_LINE8, -VGA_DRAW_LINE15, -VGA_DRAW_LINE16, -VGA_DRAW_LINE24, -VGA_DRAW_LINE32, +VGA_DRAW_LINE15_LE, +VGA_DRAW_LINE16_LE, +VGA_DRAW_LINE24_LE, +VGA_DRAW_LINE32_LE, +VGA_DRAW_LINE15_BE, +VGA_DRAW_LINE16_BE, +VGA_DRAW_LINE24_BE, +VGA_DRAW_LINE32_BE, VGA_DRAW_LINE_NB, }; @@ -1388,10 +1392,14 @@ static vga_draw_line_func * const vga_draw_line_table[VGA_DRAW_LINE_NB] = { vga_draw_line4d2, vga_draw_line8d2, vga_draw_line8, -vga_draw_line15, -vga_draw_line16, -vga_draw_line24, -vga_draw_line32, +vga_draw_line15_le, +vga_draw_line16_le, +vga_draw_line24_le, +vga_draw_line32_le, +vga_draw_line15_be, +vga_draw_line16_be, +vga_draw_line24_be, +vga_draw_line32_be, }; static int vga_get_bpp(VGACommonState *s) @@ -1464,10 +1472,15 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) uint8_t *d; uint32_t v, addr1, addr; vga_draw_line_func *vga_draw_line; -#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) -static const bool byteswap = false; +#if defined(TARGET_WORDS_BIGENDIAN) +static const bool big_endian_fb = true; #else -static const bool byteswap = true; +static const bool big_endian_fb = false; +#endif +#if defined(HOST_WORDS_BIGENDIAN) +static const bool byteswap = !big_endian_fb; +#else +static const bool byteswap = big_endian_fb; #endif full_update |= update_basic_params(s); @@ -1572,19 +1585,19 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) bits = 8; break; case 15: -v = VGA_DRAW_LINE15; +v = big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; bits = 16; break; case 16: -v = VGA_DRAW_LINE16; +v = big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; bits = 16; break; case 24: -v = VGA_DRAW_LINE24; +v = big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; bits = 24; break; case 32: -v = VGA_DRAW_LINE32; +v = big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; bits = 32; break; } diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h index 0660b52..94f6de2 100644 --- a/hw/display/vga_template.h +++ b/hw/display/vga_template.h @@ -278,21 +278,36 @@ static void vga_draw_line8(VGACommonState *s1, uint8_t *d, } } - -/* XXX: optimize */ - /* * 15 bit color */ -static void vga_draw_line15(VGACommonState *s1, uint8_t *d, -const uint8_t *s, int width) +static void vga_draw_line15_le(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) { int w; uint32_t v, r, g, b; w = width; do { -v = lduw_p((void *)s); +v = lduw_le_p((void *)s); +r = (v >> 7) & 0xf8; +g = (v >> 2) & 0xf8; +b = (v << 3) & 0xf8; +((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b); +s += 2; +d += 4; +} while (--w != 0); +} + +static void vga_draw_line15_be(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) +{ +int w; +uint32_t v, r, g, b; + +w = width; +do { +v = lduw_be_p((void *)s); r = (v >> 7) & 0xf8; g = (v >> 2) & 0xf8; b = (v << 3) & 0xf8; @@ -305,15 +320,33 @@ static void vga_draw_line15(VGACommonState *s1, uint8_t *d, /* * 16 bit color */ -static void vga_draw_line16(VGACommonState *s1, uint8_t *d, -const uint8_t *s, int width) +static void vga_draw_line16_le(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) { int w; uint32_t v, r, g, b; w = width; do { -v = lduw_p((void *)s); +v = lduw_le_p((void *)s); +r = (v >> 8) & 0xf8; +g = (v >> 3) & 0xfc; +b = (v << 3) & 0xf8; +((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b); +s += 2; +d += 4; +}
[Qemu-devel] [PATCH v2 10/11] vga: Add endian control register
From: Benjamin Herrenschmidt Include the endian state in the migration stream as an optional subsection which we only include when the endian isn't the default, thus enabling backward compatibility of the common case. Signed-off-by: Benjamin Herrenschmidt Changes by kraxel: * Remove bochs dispi interface changes. We'll do that in a different way to make sure we don't conflict with possible future bochs dispi interface changes. * keep live migration bits. Signed-off-by: Gerd Hoffmann --- hw/display/vga.c | 41 + hw/display/vga_int.h | 2 ++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 49a4b8b..19e7f23 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1508,7 +1508,8 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) if (s->line_offset != s->last_line_offset || disp_width != s->last_width || height != s->last_height || -s->last_depth != depth) { +s->last_depth != depth || +s->last_byteswap != byteswap) { if (depth == 32 || (depth == 16 && !byteswap)) { pixman_format_code_t format = qemu_default_pixman_format(depth, !byteswap); @@ -1526,6 +1527,7 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) s->last_height = height; s->last_line_offset = s->line_offset; s->last_depth = depth; +s->last_byteswap = byteswap; full_update = 1; } else if (is_buffer_shared(surface) && (full_update || surface_data(surface) != s->vram_ptr @@ -1789,6 +1791,7 @@ void vga_common_reset(VGACommonState *s) s->cursor_start = 0; s->cursor_end = 0; s->cursor_offset = 0; +s->big_endian_fb = s->default_endian_fb; memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table)); memset(s->last_palette, '\0', sizeof(s->last_palette)); memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr)); @@ -2020,6 +2023,28 @@ static int vga_common_post_load(void *opaque, int version_id) return 0; } +static bool vga_endian_state_needed(void *opaque) +{ +VGACommonState *s = opaque; + +/* + * Only send the endian state if it's different from the + * default one, thus ensuring backward compatibility for + * migration of the common case + */ +return s->default_endian_fb != s->big_endian_fb; +} + +const VMStateDescription vmstate_vga_endian = { +.name = "vga.endian", +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_BOOL(big_endian_fb, VGACommonState), +VMSTATE_END_OF_LIST() +} +}; + const VMStateDescription vmstate_vga_common = { .name = "vga", .version_id = 2, @@ -2056,6 +2081,14 @@ const VMStateDescription vmstate_vga_common = { VMSTATE_UINT32(vbe_line_offset, VGACommonState), VMSTATE_UINT32(vbe_bank_mask, VGACommonState), VMSTATE_END_OF_LIST() +}, +.subsections = (VMStateSubsection []) { +{ +.vmsd = &vmstate_vga_endian, +.needed = vga_endian_state_needed, +}, { +/* empty */ +} } }; @@ -2126,14 +2159,14 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) } /* - * Set default fb endian based on target, should probably be turned + * Set default fb endian based on target, could probably be turned * into a device attribute set by the machine/platform to remove * all target endian dependencies from this file. */ #ifdef TARGET_WORDS_BIGENDIAN -s->big_endian_fb = true; +s->default_endian_fb = true; #else -s->big_endian_fb = false; +s->default_endian_fb = false; #endif vga_dirty_log_start(s); } diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 28d67cf..ed69e06 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -150,6 +150,7 @@ typedef struct VGACommonState { uint32_t last_width, last_height; /* in chars or pixels */ uint32_t last_scr_width, last_scr_height; /* in pixels */ uint32_t last_depth; /* in bits */ +bool last_byteswap; uint8_t cursor_start, cursor_end; bool cursor_visible_phase; int64_t cursor_blink_time; @@ -158,6 +159,7 @@ typedef struct VGACommonState { bool full_update_text; bool full_update_gfx; bool big_endian_fb; +bool default_endian_fb; /* hardware mouse cursor support */ uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; void (*cursor_invalidate)(struct VGACommonState *s); -- 1.8.3.1
Re: [Qemu-devel] [PATCH V3 7/7] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
Hi Igor, On 09/26/2014 09:37 PM, Igor Mammedov wrote: > On Wed, 17 Sep 2014 09:24:03 +0800 > Gu Zheng wrote: > >> Introduce help function acpi_set_local_sts() to simplify acpi_cpu_plug_cb >> and acpi_cpu_hotplug_init, so that we can keep bit setting in one place. >> >> Signed-off-by: Gu Zheng >> --- >> hw/acpi/cpu_hotplug.c | 22 +++--- >> 1 files changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c >> index 7629686..d42c961 100644 >> --- a/hw/acpi/cpu_hotplug.c >> +++ b/hw/acpi/cpu_hotplug.c >> @@ -36,8 +36,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = { >> }, >> }; >> >> -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, >> - AcpiCpuHotplug *g, CPUState *cpu, Error **errp) >> +static void acpi_set_local_sts(AcpiCpuHotplug *g, CPUState *cpu, >> + Error **errp) >> { >> CPUClass *k = CPU_GET_CLASS(cpu); >> int64_t cpu_id; >> @@ -48,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, >> return; >> } >> >> -ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; >> g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); >> +} >> >> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, >> + AcpiCpuHotplug *g, CPUState *cpu, Error **errp) >> +{ >> +acpi_set_local_sts(g, cpu, errp); >> +if (*errp != NULL) { >> +return; >> +} >> + > >> +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; > ^^^ shouldn't be set for coldplugged CPUs along with IRQ below > >> acpi_update_sci(ar, irq); >> } >> >> @@ -60,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object >> *owner, >> CPUState *cpu; >> >> CPU_FOREACH(cpu) { >> -CPUClass *cc = CPU_GET_CLASS(cpu); >> -int64_t id = cc->get_arch_id(cpu); >> +Error *local_err = NULL; >> >> -g_assert((id / 8) < ACPI_GPE_PROC_LEN); >> -gpe_cpu->sts[id / 8] |= (1 << (id % 8)); >> +acpi_set_local_sts(gpe_cpu, cpu, &local_err); >> +g_assert(local_err == NULL); > Wouldn't acpi_cpu_plug_cb() do everything this FOREACH does? > I've suggest to drop CPU_FOREACH() altogether. I got your meaning: Make acpi_cpu_plug_cb serve all CPUs (both startup and hotpluged) , and only triggers sci irq for hotpluged ones, so that we can drop the duplicated operations in acpi_cpu_hotplug_init. But one problem is that pcms->acpi_dev was registered after startup CPUs set realized, so acpi_cpu_plug_cb can not serve startup CPUs. I tried to switch the order, but it failed, because there are some internal dependences. Now, I'm trying to split the startup CPUs init into two steps (init and realize), and register pcms->acpi_dev between the two steps. What's your opinion? Thanks, Gu > >> } >> memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops, >>gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN); > > . >
[Qemu-devel] [PATCH v5 0/2] In memory QEMUFile
From: "Dr. David Alan Gilbert" This patch-pair adds the QEMUSizedBuffer based in-memory QEMUFile written by Stefan Berger and Joel Schopp. I've made some fixes and modified the existing test-vmstate to use it for some test cases. While there's nothing other than test cases using it yet, I think it's worth going in by itself, since I'm using it in two separate patchsets (postcopy and visitor/BER) and Sanidhya uses it in the periodic vmstate test world. In addition both microcheckpointing and COLO have similar but independent implementations (although they both have some extra-gotcha's so it might not be possible to reuse it), and there was another implementation of the same thing in the Yabusame Postcopy world. Thus it seems best to put in, if only to stop people writing yet another implementation. Dave v5: Fixes from Eric's comments; including a memory leak in an error path v4: Fix very silly mistake in qsb_grow ENOMEM check v3: Mostly addressing comments from Eric and Gonglei rewrote qsb_grow to always use _try_ for memory allocation also made it use MAX_CHUNK_SIZE as needed made QSB_MAX_CHUNK_SIZE 16x rather than 10x CHUNK_SIZE reworked qsb_get_buffer to always take an allocated buffer qsb_create: Cope with 0 length but passed buffer Rework so it can fail on memory allocation failures qsb_write_at return -EINVAL for invalid position qsb_clone Cope with qsb_write_at failing Typo: 'of of' in comment 'withing' Reword qsb_clone 'exact copy'->'deep copy' Dr. David Alan Gilbert (2): QEMUSizedBuffer based QEMUFile Tests: QEMUSizedBuffer/QEMUBuffer include/migration/qemu-file.h | 28 +++ include/qemu/typedefs.h | 1 + qemu-file.c | 456 ++ tests/Makefile| 2 +- tests/test-vmstate.c | 74 +++ 5 files changed, 524 insertions(+), 37 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v5 1/2] QEMUSizedBuffer based QEMUFile
From: "Dr. David Alan Gilbert" This is based on Stefan and Joel's patch that creates a QEMUFile that goes to a memory buffer; from: http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05036.html Using the QEMUFile interface, this patch adds support functions for operating on in-memory sized buffers that can be written to or read from. Signed-off-by: Stefan Berger Signed-off-by: Joel Schopp For fixes/tweeks I've done: Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Eric Blake --- include/migration/qemu-file.h | 28 +++ include/qemu/typedefs.h | 1 + qemu-file.c | 456 ++ 3 files changed, 485 insertions(+) diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h index c90f529..6ef8ebc 100644 --- a/include/migration/qemu-file.h +++ b/include/migration/qemu-file.h @@ -25,6 +25,8 @@ #define QEMU_FILE_H 1 #include "exec/cpu-common.h" +#include + /* This function writes a chunk of data to a file at the given position. * The pos argument can be ignored if the file is only being used for * streaming. The handler should try to write all of the data it can. @@ -94,11 +96,21 @@ typedef struct QEMUFileOps { QEMURamSaveFunc *save_page; } QEMUFileOps; +struct QEMUSizedBuffer { +struct iovec *iov; +size_t n_iov; +size_t size; /* total allocated size in all iov's */ +size_t used; /* number of used bytes */ +}; + +typedef struct QEMUSizedBuffer QEMUSizedBuffer; + QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops); QEMUFile *qemu_fopen(const char *filename, const char *mode); QEMUFile *qemu_fdopen(int fd, const char *mode); QEMUFile *qemu_fopen_socket(int fd, const char *mode); QEMUFile *qemu_popen_cmd(const char *command, const char *mode); +QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input); int qemu_get_fd(QEMUFile *f); int qemu_fclose(QEMUFile *f); int64_t qemu_ftell(QEMUFile *f); @@ -111,6 +123,22 @@ void qemu_put_byte(QEMUFile *f, int v); void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size); bool qemu_file_mode_is_not_valid(const char *mode); +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len); +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *); +void qsb_free(QEMUSizedBuffer *); +size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length); +size_t qsb_get_length(const QEMUSizedBuffer *qsb); +ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count, + uint8_t *buf); +ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf, + off_t pos, size_t count); + + +/* + * For use on files opened with qemu_bufopen + */ +const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f); + static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) { qemu_put_byte(f, (int)v); diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 5f20b0e..db1153a 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -60,6 +60,7 @@ typedef struct PCIEAERLog PCIEAERLog; typedef struct PCIEAERErr PCIEAERErr; typedef struct PCIEPort PCIEPort; typedef struct PCIESlot PCIESlot; +typedef struct QEMUSizedBuffer QEMUSizedBuffer; typedef struct MSIMessage MSIMessage; typedef struct SerialState SerialState; typedef struct PCMCIACardState PCMCIACardState; diff --git a/qemu-file.c b/qemu-file.c index a8e3912..ccc516c 100644 --- a/qemu-file.c +++ b/qemu-file.c @@ -878,3 +878,459 @@ uint64_t qemu_get_be64(QEMUFile *f) v |= qemu_get_be32(f); return v; } + +#define QSB_CHUNK_SIZE (1 << 10) +#define QSB_MAX_CHUNK_SIZE (16 * QSB_CHUNK_SIZE) + +/** + * Create a QEMUSizedBuffer + * This type of buffer uses scatter-gather lists internally and + * can grow to any size. Any data array in the scatter-gather list + * can hold different amount of bytes. + * + * @buffer: Optional buffer to copy into the QSB + * @len: size of initial buffer; if @buffer is given, buffer must + * hold at least len bytes + * + * Returns a pointer to a QEMUSizedBuffer or NULL on allocation failure + */ +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len) +{ +QEMUSizedBuffer *qsb; +size_t alloc_len, num_chunks, i, to_copy; +size_t chunk_size = (len > QSB_MAX_CHUNK_SIZE) +? QSB_MAX_CHUNK_SIZE +: QSB_CHUNK_SIZE; + +num_chunks = DIV_ROUND_UP(len ? len : QSB_CHUNK_SIZE, chunk_size); +alloc_len = num_chunks * chunk_size; + +qsb = g_try_new0(QEMUSizedBuffer, 1); +if (!qsb) { +return NULL; +} + +qsb->iov = g_try_new0(struct iovec, num_chunks); +if (!qsb->iov) { +g_free(qsb); +return NULL; +} + +qsb->n_iov = num_chunks; + +for (i = 0; i < num_chunks; i++) { +qsb->iov[i].iov_base = g_try_malloc0(chunk_size); +if (!qsb->iov[i].iov_base) { +/* qsb_free is safe since g_free can cope with NULL */ +qsb_free(qsb); +
[Qemu-devel] [PATCH v5 2/2] Tests: QEMUSizedBuffer/QEMUBuffer
From: "Dr. David Alan Gilbert" Modify some of tests/test-vmstate.c to use the in memory file based on QEMUSizedBuffer to provide basic testing of QEMUSizedBuffer and the associated memory backed QEMUFile type. Only some of the tests are changed so that the fd backed QEMUFile is still tested. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Eric Blake --- tests/Makefile | 2 +- tests/test-vmstate.c | 74 +++- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index f5de29c..0ad7ac5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -259,7 +259,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \ libqemuutil.a libqemustub.a tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ vmstate.o qemu-file.o \ - libqemuutil.a + libqemuutil.a libqemustub.a tests/test-qapi-types.c tests/test-qapi-types.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c index d72c64c..5e0fd13 100644 --- a/tests/test-vmstate.c +++ b/tests/test-vmstate.c @@ -43,6 +43,12 @@ void yield_until_fd_readable(int fd) select(fd + 1, &fds, NULL, NULL, NULL); } +/* + * Some tests use 'open_test_file' to work on a real fd, some use + * an in memory file (QEMUSizedBuffer+qemu_bufopen); we could pick one + * but this way we test both. + */ + /* Duplicate temp_fd and seek to the beginning of the file */ static QEMUFile *open_test_file(bool write) { @@ -54,6 +60,30 @@ static QEMUFile *open_test_file(bool write) return qemu_fdopen(fd, write ? "wb" : "rb"); } +/* Open a read-only qemu-file from an existing memory block */ +static QEMUFile *open_mem_file_read(const void *data, size_t len) +{ +/* The qsb gets freed by qemu_fclose */ +QEMUSizedBuffer *qsb = qsb_create(data, len); +g_assert(qsb); + +return qemu_bufopen("r", qsb); +} + +/* + * Check that the contents of the memory-buffered file f match + * the given size/data. + */ +static void check_mem_file(QEMUFile *f, void *data, size_t size) +{ +uint8_t *result = g_malloc(size); +const QEMUSizedBuffer *qsb = qemu_buf_get(f); +g_assert_cmpint(qsb_get_length(qsb), ==, size); +g_assert_cmpint(qsb_get_buffer(qsb, 0, size, result), ==, size); +g_assert_cmpint(memcmp(result, data, size), ==, 0); +g_free(result); +} + #define SUCCESS(val) \ g_assert_cmpint((val), ==, 0) @@ -371,14 +401,12 @@ static const VMStateDescription vmstate_skipping = { static void test_save_noskip(void) { -QEMUFile *fsave = open_test_file(true); +QEMUFile *fsave = qemu_bufopen("w", NULL); TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6, .skip_c_e = false }; vmstate_save_state(fsave, &vmstate_skipping, &obj); g_assert(!qemu_file_get_error(fsave)); -qemu_fclose(fsave); -QEMUFile *loading = open_test_file(false); uint8_t expected[] = { 0, 0, 0, 1, /* a */ 0, 0, 0, 2, /* b */ @@ -387,52 +415,31 @@ static void test_save_noskip(void) 0, 0, 0, 5, /* e */ 0, 0, 0, 0, 0, 0, 0, 6, /* f */ }; -uint8_t result[sizeof(expected)]; -g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==, -sizeof(result)); -g_assert(!qemu_file_get_error(loading)); -g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0); - -/* Must reach EOF */ -qemu_get_byte(loading); -g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO); - -qemu_fclose(loading); +check_mem_file(fsave, expected, sizeof(expected)); +qemu_fclose(fsave); } static void test_save_skip(void) { -QEMUFile *fsave = open_test_file(true); +QEMUFile *fsave = qemu_bufopen("w", NULL); TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6, .skip_c_e = true }; vmstate_save_state(fsave, &vmstate_skipping, &obj); g_assert(!qemu_file_get_error(fsave)); -qemu_fclose(fsave); -QEMUFile *loading = open_test_file(false); uint8_t expected[] = { 0, 0, 0, 1, /* a */ 0, 0, 0, 2, /* b */ 0, 0, 0, 0, 0, 0, 0, 4, /* d */ 0, 0, 0, 0, 0, 0, 0, 6, /* f */ }; -uint8_t result[sizeof(expected)]; -g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==, -sizeof(result)); -g_assert(!qemu_file_get_error(loading)); -g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0); - - -/* Must reach EOF */ -qemu_get_byte(loading); -g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO); +check_mem_file(fsave, expected, sizeof(expected)); -qemu_fclose(loading); +qemu_fclose(fsave); } static void test_load_noskip(void) { -QEMUFile *fsave = open_test_file(
Re: [Qemu-devel] [PATCH V3 7/7] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
On Mon, 29 Sep 2014 17:22:08 +0800 Gu Zheng wrote: > Hi Igor, > On 09/26/2014 09:37 PM, Igor Mammedov wrote: > > > On Wed, 17 Sep 2014 09:24:03 +0800 > > Gu Zheng wrote: > > > >> Introduce help function acpi_set_local_sts() to simplify > >> acpi_cpu_plug_cb and acpi_cpu_hotplug_init, so that we can keep > >> bit setting in one place. > >> > >> Signed-off-by: Gu Zheng > >> --- > >> hw/acpi/cpu_hotplug.c | 22 +++--- > >> 1 files changed, 15 insertions(+), 7 deletions(-) > >> > >> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c > >> index 7629686..d42c961 100644 > >> --- a/hw/acpi/cpu_hotplug.c > >> +++ b/hw/acpi/cpu_hotplug.c > >> @@ -36,8 +36,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops > >> = { }, > >> }; > >> > >> -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, > >> - AcpiCpuHotplug *g, CPUState *cpu, Error > >> **errp) +static void acpi_set_local_sts(AcpiCpuHotplug *g, > >> CPUState *cpu, > >> + Error **errp) > >> { > >> CPUClass *k = CPU_GET_CLASS(cpu); > >> int64_t cpu_id; > >> @@ -48,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq > >> irq, return; > >> } > >> > >> -ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; > >> g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); > >> +} > >> > >> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, > >> + AcpiCpuHotplug *g, CPUState *cpu, Error > >> **errp) +{ > >> +acpi_set_local_sts(g, cpu, errp); > >> +if (*errp != NULL) { > >> +return; > >> +} > >> + > > > >> +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; > > ^^^ shouldn't be set for coldplugged CPUs along with IRQ below > > > >> acpi_update_sci(ar, irq); > >> } > >> > >> @@ -60,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion > >> *parent, Object *owner, CPUState *cpu; > >> > >> CPU_FOREACH(cpu) { > >> -CPUClass *cc = CPU_GET_CLASS(cpu); > >> -int64_t id = cc->get_arch_id(cpu); > >> +Error *local_err = NULL; > >> > >> -g_assert((id / 8) < ACPI_GPE_PROC_LEN); > >> -gpe_cpu->sts[id / 8] |= (1 << (id % 8)); > >> +acpi_set_local_sts(gpe_cpu, cpu, &local_err); > >> +g_assert(local_err == NULL); > > Wouldn't acpi_cpu_plug_cb() do everything this FOREACH does? > > I've suggest to drop CPU_FOREACH() altogether. > > I got your meaning: Make acpi_cpu_plug_cb serve all CPUs (both > startup and hotpluged) , and only triggers sci irq for hotpluged > ones, so that we can drop the duplicated operations in > acpi_cpu_hotplug_init. But one problem is that pcms->acpi_dev was > registered after startup CPUs set realized, so acpi_cpu_plug_cb can > not serve startup CPUs. I tried to switch the order, but it failed, > because there are some internal dependences. Now, I'm trying to split > the startup CPUs init into two steps (init and realize), and register > pcms->acpi_dev between the two steps. What's your opinion? Could you point out what dependecies are there? > > Thanks, > Gu > > > > >> } > >> memory_region_init_io(&gpe_cpu->io, owner, > >> &AcpiCpuHotplug_ops, gpe_cpu, "acpi-cpu-hotplug", > >> ACPI_GPE_PROC_LEN); > > > > . > > > >
Re: [Qemu-devel] [PATCH bugfix v2] snapshot: fix referencing wrong variable in while loop in do_delvm
Am 29.09.2014 um 10:38 hat Zhang Haoyu geschrieben: > The while loop variabal is "bs1", > but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. > Broken in commit a89d89d, v1.7.0. > > v1 -> v2: > * add broken commit id to commit message > > Signed-off-by: Zhang Haoyu > Reviewed-by: Markus Armbruster Cc: qemu-sta...@nongnu.org Reviewed-by: Kevin Wolf We should probably add a qemu-iotests case for the handling of internal snapshots on VMs with multiple images. Any volunteer? Kevin
Re: [Qemu-devel] [PATCH bugfix v2] snapshot: fix referencing wrong variable in while loop in do_delvm
On Mon, Sep 29, 2014 at 11:18:33AM +0200, Markus Armbruster wrote: > > The while loop variabal is "bs1", > > but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. > > Broken in commit a89d89d, v1.7.0. > > > > v1 -> v2: > > * add broken commit id to commit message > > Patch version information... > > > Signed-off-by: Zhang Haoyu > > Reviewed-by: Markus Armbruster > > --- > > ... goes here, so it doesn't go into the permanent commit message. > Maybe Kevin or Stefan can clean it up on commit. No problem, we'll drop the changelog when applying the patch this time. Thanks for reviewing, Markus. pgpvclG5Q_25D.pgp Description: PGP signature
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough
On Sun, Sep 28, 2014 at 10:59:05AM +0800, Chen, Tiejun wrote: > On 2014/9/3 9:40, Kay, Allen M wrote: > > > > > >>-Original Message- > >>From: Chen, Tiejun > >>Sent: Monday, September 01, 2014 12:50 AM > >>To: Michael S. Tsirkin > >>Cc: xen-de...@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org; > >>Konrad Rzeszutek Wilk > >>Subject: Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create > >>isa bridge specific to IGD passthrough > >> > >>On 2014/9/1 14:05, Michael S. Tsirkin wrote: > >>>On Mon, Sep 01, 2014 at 10:50:37AM +0800, Chen, Tiejun wrote: > On 2014/8/31 16:58, Michael S. Tsirkin wrote: > >On Fri, Aug 29, 2014 at 09:28:50AM +0800, Chen, Tiejun wrote: > >> > >> > >>On 2014/8/28 8:56, Chen, Tiejun wrote: > >+ */ > >+dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0), > >+"xen-igd-passthrough-isa-bridge"); > >+if (dev) { > >+r = xen_host_pci_device_get(&hdev, 0, 0, > >+ PCI_DEVFN(0x1f, > >0), 0); > >+if (!r) { > >+pci_config_set_vendor_id(dev->config, > >>hdev.vendor_id); > >+pci_config_set_device_id(dev->config, > >+ hdev.device_id); > >> > >>Can you, instead, implement the reverse logic, probing the card > >>and supplying the correct device id for PCH? > >> > > > >Here what is your so-called reverse logic as I already asked you > >previously? Do you mean I should list all PCHs with a combo > >illustrated with the vendor/device id in advance? Then look up > >if we can find a > > Michael, > > >>> > >>>Ping. > >>> > >>>Thanks > >>>Tiejun > >>> > Could you explain this exactly? Then I can try follow-up your > idea ASAP if this is necessary and possible. > >> > >>Michel, > >> > >>Could you give us some explanation for your "reverse logic" when > >>you're free? > >> > >>Thanks > >>Tiejun > > > >So future drivers will look at device ID for the card and figure out > >how things should work from there. > >Old drivers still poke at device id of the chipset for this, but > >maybe qemu can do what newer drivers do: > >look at the card and figure out what guest should do, then present > >the appropriate chipset id. > > > >This is based on what Jesse said: > > Practically speaking, we could probably assume specific GPU/PCH > >>combos, > > as I don't think they're generally mixed across generations, though > >>SNB > > and IVB did have compatible sockets, so there is the possibility of > > mixing CPT and PPT PCHs, but those are register identical as far as the > > graphics driver is concerned, so even that should be safe. > > > > Michael, > > Thanks for your explanation. > > >so the idea is to have a reverse table mapping GPU back to PCH. > >Present to guest the ID that will let it assume the correct GPU. > > I guess you mean we should create to maintain such a table: > > [GPU Card: device_id(s), PCH: device_id] > > Then each time, instead of exposing that real PCH device id directly, > qemu first can get the real GPU device id to lookup this table to > present a appropriate PCH's device id to the guest. > > And looks here that appropriate PCH's device id is not always a that > real PCH's device id. Right? If I'm wrong please correct me. > >>> > >>>Exactly: we don't really care what the PCH ID is - we only need it for > >>>the guest driver to do the right thing. > >> > >>Agreed. > >> > >>I need to ask Allen to check if one given GPU card device id is always > >>corresponding to one given PCH on both HSW and BDW currently. If yes, I can > >>do this quickly. Otherwise I need some time to establish that sort > >>of connection. > > Michael, > > Sorry for this delay response but please keep in mind we really are in this > process. > > You know this involve different GPU components we have to take some time to > communicate or even discuss internal. > > Now I have a draft codes, could you take a look at this? I hope that comment > can help us to understand something, but if you have any question, we can > further respond inline. > > --- > typedef struct { > uint16_t gpu_device_id; > uint16_t pch_device_id; > } XenIGDDeviceIDInfo; > > /* In real world different GPU should have different PCH. But actually > * the different PCH DIDs likely map to different PCH SKUs. We do the > * same thing for the GPU. For PCH, the different SKUs are going to be > * all the same silicon design and implementation, just different > * features turn on and off with fuses. The SW interfaces should be > * consistent across all SKUs in a given family (eg LPT). But just sam
Re: [Qemu-devel] [PATCH V3 7/7] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
On 09/29/2014 05:47 PM, Igor Mammedov wrote: > On Mon, 29 Sep 2014 17:22:08 +0800 > Gu Zheng wrote: > >> Hi Igor, >> On 09/26/2014 09:37 PM, Igor Mammedov wrote: >> >>> On Wed, 17 Sep 2014 09:24:03 +0800 >>> Gu Zheng wrote: >>> Introduce help function acpi_set_local_sts() to simplify acpi_cpu_plug_cb and acpi_cpu_hotplug_init, so that we can keep bit setting in one place. Signed-off-by: Gu Zheng --- hw/acpi/cpu_hotplug.c | 22 +++--- 1 files changed, 15 insertions(+), 7 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 7629686..d42c961 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -36,8 +36,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = { }, }; -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, - AcpiCpuHotplug *g, CPUState *cpu, Error **errp) +static void acpi_set_local_sts(AcpiCpuHotplug *g, CPUState *cpu, + Error **errp) { CPUClass *k = CPU_GET_CLASS(cpu); int64_t cpu_id; @@ -48,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, return; } -ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); +} +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, CPUState *cpu, Error **errp) +{ +acpi_set_local_sts(g, cpu, errp); +if (*errp != NULL) { +return; +} + >>> +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; >>> ^^^ shouldn't be set for coldplugged CPUs along with IRQ below >>> acpi_update_sci(ar, irq); } @@ -60,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, CPUState *cpu; CPU_FOREACH(cpu) { -CPUClass *cc = CPU_GET_CLASS(cpu); -int64_t id = cc->get_arch_id(cpu); +Error *local_err = NULL; -g_assert((id / 8) < ACPI_GPE_PROC_LEN); -gpe_cpu->sts[id / 8] |= (1 << (id % 8)); +acpi_set_local_sts(gpe_cpu, cpu, &local_err); +g_assert(local_err == NULL); >>> Wouldn't acpi_cpu_plug_cb() do everything this FOREACH does? >>> I've suggest to drop CPU_FOREACH() altogether. >> >> I got your meaning: Make acpi_cpu_plug_cb serve all CPUs (both >> startup and hotpluged) , and only triggers sci irq for hotpluged >> ones, so that we can drop the duplicated operations in >> acpi_cpu_hotplug_init. But one problem is that pcms->acpi_dev was >> registered after startup CPUs set realized, so acpi_cpu_plug_cb can >> not serve startup CPUs. I tried to switch the order, but it failed, >> because there are some internal dependences. Now, I'm trying to split >> the startup CPUs init into two steps (init and realize), and register >> pcms->acpi_dev between the two steps. What's your opinion? > Could you point out what dependecies are there? E.g. Allocation smi_irq for PIIX4PMState dependents on the valid *first_cpu*. smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1); /* TODO: Populate SPD eeprom data. */ smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, gsi[9], *smi_irq, kvm_enabled(), fw_cfg, &piix4_pm); Thanks, Gu > >> >> Thanks, >> Gu >> >>> } memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops, gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN); >>> >>> . >>> >> >> > > . >
Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm
On Sun, Sep 28, 2014 at 09:33:08PM +0100, Alex Bligh wrote: > Hang on a second! v2 of this patch DID use a new virtual machine, > called exactly that. I thought you were objecting to that and > wanting a machine parameter instead! It's far easier with a new > machine type, and I'd far prefer a new machine type. > > If you were just objecting to the fact that pc-1.0 was made to > be an alias of either one or the other at compile time, simply > drop the second patch of the v2 patchset. I think same applies to v3 that I reviewed right? Absolutely, I'm fine with just a new machine type. This means that management tools will need to learn to add -qemu-kvm suffix to the machine name if user requested compatibility with qemu-kvm. I think there were some implementation issues with patch 1/2 though. > If we have a new machine type, I don't /think/ I need the early_init > thing at all (I may be wrong about that). Good. > -- > Alex Bligh > > >
[Qemu-devel] build regression on osx in block layer
Build fails with: $ make LINK qemu-nbd Undefined symbols for architecture x86_64: "_posix_fallocate", referenced from: _raw_create in raw-posix.o ld: symbol(s) not found for architecture x86_64 introduced by 06247428be raw-posix: Add falloc and full preallocation
Re: [Qemu-devel] [PATCH v4 7/7] virtio-scsi: Handle TMF request cancellation asynchronously
Il 28/09/2014 03:48, Fam Zheng ha scritto: > +virtio_scsi_complete_req(req); > +} else { > +assert(r = -EINPROGRESS); > +} > } = instead of == here. Apart from this, the patch looks good. Thanks! Paolo
Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API
On Fri, Sep 26, 2014 at 09:28:05AM +, Igor Mammedov wrote: > Changes since v1: > * added usb-uas test > * drop hotplug check in bus_add_child() > * make SCSI & USB bus as default HotplugHandler > * fixed dummy HBA hot(un)plug > * fixed hotunplug on s390x > * prevent hotplug of non hotpluggable devices For pci, virtio, acpi bits: Reviewed-by: Michael S. Tsirkin Which tree should this go in through? afaerber's? In that case, it might be a good idea to Cc him ... > -- > > Series unifies different hotplug mechanisms to a recent > hotplug-handler API and does shallow conversion of > devices that still use legacy qdev hotplug to it dropping > not used after that legacy hotplug path [29/30]. > It also relaces SCSI's own way to do hotplug/unplug with > hotplug-handler callbacks leaving it the only method > perform hotplug tasks. > And the last patch [30/30] allows to unplug of BUS-less > devices using hotplug-handler API. > > Converted devices are covered with new hotplug > unit-tests, except of: > s390x-*: I have no idea how or means to test it, but code > is close to virtio, so it's converted using > virtio template > pvscsi: is broken, so no means to test it > > Git tree for testing: > https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2 > > > Igor Mammedov (36): > test: virtio-scsi: check if hot-plug/unplug works > test: virtio-serial: check if hot-plug/unplug works > test: libqos: add qpci_plug_device_test() and > qpci_unplug_acpi_device_test() > test: virtio-rng: check if hot-plug/unplug works > test: virtio-net: check if hot-plug/unplug works > test: virtio-blk: check if hot-plug/unplug works > test: usb: move uhci port test code to libqos/usb.c > test: usb: add port test to uhci unit test > test: usb: generic usb device hotplug > test: usb: usb-storage hotplug test > test: usb: usb-uas hotplug test > access BusState.allow_hotplug using wraper qbus_is_hotpluggable() > qdev: do not allow to instantiate non hotpluggable device with > device_add > qdev: HotplugHandler: rename unplug callback to unplug_request > qdev: HotplugHandler: provide unplug callback > qdev: add simple/generic unplug callback for HotplugHandler > qdev: add wrapper to set BUS as HotplugHandler > qdev: drop hotplug check from bus_add_child() > target-i386: ICC bus: drop BusState.allow_hotplug > virtio-pci: drop BusState.allow_hotplug > virtio-serial: convert to hotplug-handler API > virtio-mmio: drop useless bus->allow_hotplug = 0 > s390x: drop not used allow_hotplug in event-facility > s390x: convert s390-virtio to hotplug handler API > s390x: convert virtio-ccw to hotplug handler API > scsi: set SCSI BUS itself as default HotplugHandler > scsi: convert pvscsi HBA to hotplug handler API > scsi: convert virtio-scsi HBA to hotplug handler API > scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} fields > usb-bot: mark device as non hotpluggable > usb-bot: drop not needed "allow_hotplug = 0" > usb-storage: drop not needed "allow_hotplug = 0" > usb: convert usb-ccid to hotplug handler API > usb: convert usb devices to hotplug handler API > qdev: drop legacy hotplug fields/methods > qdev: HotplugHandler: add support for unplugging BUS-less devices > > hw/acpi/piix4.c| 6 +-- > hw/char/virtio-serial-bus.c| 20 +++--- > hw/core/hotplug.c | 11 ++ > hw/core/qdev.c | 85 > -- > hw/cpu/icc_bus.c | 8 > hw/i386/acpi-build.c | 2 +- > hw/isa/lpc_ich9.c | 6 +-- > hw/pci-bridge/pci_bridge_dev.c | 2 +- > hw/pci/pci-hotplug-old.c | 4 +- > hw/pci/pcie.c | 4 +- > hw/pci/pcie_port.c | 2 +- > hw/pci/shpc.c | 4 +- > hw/s390x/event-facility.c | 2 - > hw/s390x/s390-virtio-bus.c | 12 +++--- > hw/s390x/virtio-ccw.c | 17 + > hw/scsi/scsi-bus.c | 24 > hw/scsi/virtio-scsi.c | 30 ++- > hw/scsi/vmw_pvscsi.c | 26 + > hw/usb/bus.c | 9 - > hw/usb/dev-smartcard-reader.c | 8 +++- > hw/usb/dev-storage.c | 4 +- > hw/virtio/virtio-mmio.c| 17 + > hw/virtio/virtio-pci.c | 3 -- > include/hw/hotplug.h | 16 +++- > include/hw/pci/pcie.h | 4 +- > include/hw/pci/shpc.h | 4 +- > include/hw/qdev-core.h | 17 + > include/hw/scsi/scsi.h | 2 - > qdev-monitor.c | 5 ++- > tests/Makefile | 13 --- > tests/libqos/pci-pc.c | 49 > tests/libqos/pci.h | 3 ++ > tests/libqos/usb.c | 71 +++ > tests/libqos/usb.h | 17 + > tests/usb-hcd-ehci-test.c | 50 ++-
Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm
On 29 Sep 2014, at 11:08, Michael S. Tsirkin wrote: > On Sun, Sep 28, 2014 at 09:33:08PM +0100, Alex Bligh wrote: >> Hang on a second! v2 of this patch DID use a new virtual machine, >> called exactly that. I thought you were objecting to that and >> wanting a machine parameter instead! It's far easier with a new >> machine type, and I'd far prefer a new machine type. >> >> If you were just objecting to the fact that pc-1.0 was made to >> be an alias of either one or the other at compile time, simply >> drop the second patch of the v2 patchset. > > I think same applies to v3 that I reviewed right? > Absolutely, I'm fine with just a new machine type. > This means that management tools will need to learn to > add -qemu-kvm suffix to the machine name if user > requested compatibility with qemu-kvm. > I think there were some implementation issues with patch 1/2 > though. > >> If we have a new machine type, I don't /think/ I need the early_init >> thing at all (I may be wrong about that). > > Good. OK, I will respin this when I get a chance with the new machine type back and hopefully address some of the other issues you brought up. -- Alex Bligh
Re: [Qemu-devel] [PATCH bugfix v2] snapshot: fix referencing wrong variable in while loop in do_delvm
On Mon, Sep 29, 2014 at 04:38:02PM +0800, Zhang Haoyu wrote: > The while loop variabal is "bs1", > but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. > Broken in commit a89d89d, v1.7.0. > > v1 -> v2: > * add broken commit id to commit message > > Signed-off-by: Zhang Haoyu > Reviewed-by: Markus Armbruster > --- > savevm.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpSQOQxFRWDr.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
On Fri, 26 Sep 2014, Don Slutz wrote: > This adds synchronisation of the vcpu registers > between Xen and QEMU. > > Signed-off-by: Don Slutz [...] > diff --git a/xen-hvm.c b/xen-hvm.c > index 05e522c..e1274bb 100644 > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -857,14 +857,48 @@ static void cpu_handle_ioreq(void *opaque) > > handle_buffered_iopage(state); > if (req) { > +#ifdef IOREQ_TYPE_VMWARE_PORT Is there any reason to #ifdef this code? Couldn't we just always build it in QEMU? > +if (req->type == IOREQ_TYPE_VMWARE_PORT) { I think it would make more sense to check for IOREQ_TYPE_VMWARE_PORT from within handle_ioreq. > +CPUX86State *env; > +ioreq_t fake_req = { > +.type = IOREQ_TYPE_PIO, > +.addr = (uint16_t)req->size, > +.size = 4, > +.dir = IOREQ_READ, > +.df = 0, > +.data_is_ptr = 0, > +}; > +if (!xen_opaque_env) { > +xen_opaque_env = g_malloc(sizeof(CPUX86State)); > +} > +env = xen_opaque_env; > +env->regs[R_EAX] = (uint32_t)(req->addr >> 32); > +env->regs[R_EBX] = (uint32_t)(req->addr); > +env->regs[R_ECX] = req->count; > +env->regs[R_EDX] = req->size; > +env->regs[R_ESI] = (uint32_t)(req->data >> 32); > +env->regs[R_EDI] = (uint32_t)(req->data); > +handle_ioreq(&fake_req); > +req->addr = ((uint64_t)fake_req.data << 32) | > +(uint32_t)env->regs[R_EBX]; > +req->count = env->regs[R_ECX]; > +req->size = env->regs[R_EDX]; > +req->data = ((uint64_t)env->regs[R_ESI] << 32) | > +(uint32_t)env->regs[R_EDI]; This code could be moved to a separate helper function called by handle_ioreq. > +} else { > +handle_ioreq(req); > +} > +#else > handle_ioreq(req); > +#endif
Re: [Qemu-devel] [PATCH V3 7/7] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
On Mon, 29 Sep 2014 17:44:27 +0800 Gu Zheng wrote: > On 09/29/2014 05:47 PM, Igor Mammedov wrote: > > > On Mon, 29 Sep 2014 17:22:08 +0800 > > Gu Zheng wrote: > > > >> Hi Igor, > >> On 09/26/2014 09:37 PM, Igor Mammedov wrote: > >> > >>> On Wed, 17 Sep 2014 09:24:03 +0800 > >>> Gu Zheng wrote: > >>> > Introduce help function acpi_set_local_sts() to simplify > acpi_cpu_plug_cb and acpi_cpu_hotplug_init, so that we can keep > bit setting in one place. > > Signed-off-by: Gu Zheng > --- > hw/acpi/cpu_hotplug.c | 22 +++--- > 1 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c > index 7629686..d42c961 100644 > --- a/hw/acpi/cpu_hotplug.c > +++ b/hw/acpi/cpu_hotplug.c > @@ -36,8 +36,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops > = { }, > }; > > -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, > - AcpiCpuHotplug *g, CPUState *cpu, Error > **errp) +static void acpi_set_local_sts(AcpiCpuHotplug *g, > CPUState *cpu, > + Error **errp) > { > CPUClass *k = CPU_GET_CLASS(cpu); > int64_t cpu_id; > @@ -48,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq > irq, return; > } > > -ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; > g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); > +} > > +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, > + AcpiCpuHotplug *g, CPUState *cpu, Error > **errp) +{ > +acpi_set_local_sts(g, cpu, errp); > +if (*errp != NULL) { > +return; > +} > + > >>> > +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; > >>> ^^^ shouldn't be set for coldplugged CPUs along with IRQ below > >>> > acpi_update_sci(ar, irq); > } > > @@ -60,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion > *parent, Object *owner, CPUState *cpu; > > CPU_FOREACH(cpu) { > -CPUClass *cc = CPU_GET_CLASS(cpu); > -int64_t id = cc->get_arch_id(cpu); > +Error *local_err = NULL; > > -g_assert((id / 8) < ACPI_GPE_PROC_LEN); > -gpe_cpu->sts[id / 8] |= (1 << (id % 8)); > +acpi_set_local_sts(gpe_cpu, cpu, &local_err); > +g_assert(local_err == NULL); > >>> Wouldn't acpi_cpu_plug_cb() do everything this FOREACH does? > >>> I've suggest to drop CPU_FOREACH() altogether. > >> > >> I got your meaning: Make acpi_cpu_plug_cb serve all CPUs (both > >> startup and hotpluged) , and only triggers sci irq for hotpluged > >> ones, so that we can drop the duplicated operations in > >> acpi_cpu_hotplug_init. But one problem is that pcms->acpi_dev was > >> registered after startup CPUs set realized, so acpi_cpu_plug_cb can > >> not serve startup CPUs. I tried to switch the order, but it failed, > >> because there are some internal dependences. Now, I'm trying to > >> split the startup CPUs init into two steps (init and realize), and > >> register pcms->acpi_dev between the two steps. What's your opinion? > > Could you point out what dependecies are there? > > E.g. > Allocation smi_irq for PIIX4PMState dependents on the valid > *first_cpu*. > > smi_irq = qemu_allocate_irqs(pc_acpi_smi_interrupt, first_cpu, 1); > /* TODO: Populate SPD eeprom data. */ > smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, > gsi[9], *smi_irq, > kvm_enabled(), fw_cfg, &piix4_pm); Looks like it would be too much trouble for not much gain. Lets keep this patch as is. > Thanks, > Gu > > > > >> > >> Thanks, > >> Gu > >> > >>> > } > memory_region_init_io(&gpe_cpu->io, owner, > &AcpiCpuHotplug_ops, gpe_cpu, "acpi-cpu-hotplug", > ACPI_GPE_PROC_LEN); > >>> > >>> . > >>> > >> > >> > > > > . > > > > >
Re: [Qemu-devel] [PATCH v2] ssh: Don't crash if either host or path is not specified.
On Mon, Sep 29, 2014 at 09:06:22AM +0100, Richard W.M. Jones wrote: > $ ./qemu-img create -f qcow2 overlay \ > -b 'json: { "file.driver":"ssh", > "file.host":"localhost", > "file.host_key_check":"no" }' > qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' > failed. > Aborted > > A similar crash also happens if the file.host field is omitted. > > https://bugzilla.redhat.com/show_bug.cgi?id=1147343 > > Bug found and reported by Jun Li. > > Signed-off-by: Richard W.M. Jones > --- > block/ssh.c | 10 ++ > 1 file changed, 10 insertions(+) Please CC Kevin Wolf and me on block patches. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpzXzETSE1cj.pgp Description: PGP signature
[Qemu-devel] [PULL 0/2] spice patch queue
Hi, Short spice patch queue, adding a new graphic_console_set_hwops function to the console core, which in turn allows to simplify switching between vga and native mode in qxl. please pull, Gerd The following changes since commit 81ab11a7a524d12412a59ef49c6b270671e62ea0: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-09-26 15:41:50 +0100) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu tags/pull-spice-20140929-1 for you to fetch changes up to 151623353f4a3da4daec29d658c10ef3b57bd462: qxl: use graphic_console_set_hwops (2014-09-29 10:20:09 +0200) add and use graphic_console_set_hwops Gerd Hoffmann (2): console: add graphic_console_set_hwops qxl: use graphic_console_set_hwops hw/display/qxl.c | 49 + include/ui/console.h | 3 +++ ui/console.c | 11 +-- 3 files changed, 21 insertions(+), 42 deletions(-)
[Qemu-devel] [PULL 2/2] qxl: use graphic_console_set_hwops
Simply switch function pointers when entering/leaving vga mode. Allows to remove wrapper functions which do nothing but dispatch calls depending on the current qxl mode. Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 49 + 1 file changed, 9 insertions(+), 40 deletions(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 55d13a7..93b3518 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -132,6 +132,8 @@ static void qxl_reset_memslots(PCIQXLDevice *d); static void qxl_reset_surfaces(PCIQXLDevice *d); static void qxl_ring_set_dirty(PCIQXLDevice *qxl); +static void qxl_hw_update(void *opaque); + void qxl_set_guest_bug(PCIQXLDevice *qxl, const char *msg, ...) { trace_qxl_set_guest_bug(qxl->id); @@ -1076,6 +1078,10 @@ static const QXLInterface qxl_interface = { .client_monitors_config = interface_client_monitors_config, }; +static const GraphicHwOps qxl_ops = { +.gfx_update = qxl_hw_update, +}; + static void qxl_enter_vga_mode(PCIQXLDevice *d) { if (d->mode == QXL_MODE_VGA) { @@ -1085,6 +1091,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d) #if SPICE_SERVER_VERSION >= 0x000c03 /* release 0.12.3 */ spice_qxl_driver_unload(&d->ssd.qxl); #endif +graphic_console_set_hwops(d->ssd.dcl.con, d->vga.hw_ops, &d->vga); qemu_spice_create_host_primary(&d->ssd); d->mode = QXL_MODE_VGA; vga_dirty_log_start(&d->vga); @@ -1097,6 +1104,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d) return; } trace_qxl_exit_vga_mode(d->id); +graphic_console_set_hwops(d->ssd.dcl.con, &qxl_ops, d); vga_dirty_log_stop(&d->vga); qxl_destroy_primary(d, QXL_SYNC); } @@ -1756,41 +1764,8 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) static void qxl_hw_update(void *opaque) { PCIQXLDevice *qxl = opaque; -VGACommonState *vga = &qxl->vga; -switch (qxl->mode) { -case QXL_MODE_VGA: -vga->hw_ops->gfx_update(vga); -break; -case QXL_MODE_COMPAT: -case QXL_MODE_NATIVE: -qxl_render_update(qxl); -break; -default: -break; -} -} - -static void qxl_hw_invalidate(void *opaque) -{ -PCIQXLDevice *qxl = opaque; -VGACommonState *vga = &qxl->vga; - -if (qxl->mode == QXL_MODE_VGA) { -vga->hw_ops->invalidate(vga); -return; -} -} - -static void qxl_hw_text_update(void *opaque, console_ch_t *chardata) -{ -PCIQXLDevice *qxl = opaque; -VGACommonState *vga = &qxl->vga; - -if (qxl->mode == QXL_MODE_VGA) { -vga->hw_ops->text_update(vga, chardata); -return; -} +qxl_render_update(qxl); } static void qxl_dirty_surfaces(PCIQXLDevice *qxl) @@ -2049,12 +2024,6 @@ static int qxl_init_common(PCIQXLDevice *qxl) return 0; } -static const GraphicHwOps qxl_ops = { -.invalidate = qxl_hw_invalidate, -.gfx_update = qxl_hw_update, -.text_update = qxl_hw_text_update, -}; - static int qxl_init_primary(PCIDevice *dev) { PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev); -- 1.8.3.1
[Qemu-devel] [PULL 1/2] console: add graphic_console_set_hwops
Add a function to allow display emulations to switch the hwops function pointers. This is useful for devices which have two completely different operation modes. Typical case is the vga compatibility mode vs. native mode in qxl and the upcoming virtio-vga device. Signed-off-by: Gerd Hoffmann --- include/ui/console.h | 3 +++ ui/console.c | 11 +-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index cde0faf..22ef8ca 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -292,6 +292,9 @@ typedef struct GraphicHwOps { QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head, const GraphicHwOps *ops, void *opaque); +void graphic_console_set_hwops(QemuConsole *con, + const GraphicHwOps *hw_ops, + void *opaque); void graphic_hw_update(QemuConsole *con); void graphic_hw_invalidate(QemuConsole *con); diff --git a/ui/console.c b/ui/console.c index f819382..258af5d 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1677,6 +1677,14 @@ DisplayState *init_displaystate(void) return display_state; } +void graphic_console_set_hwops(QemuConsole *con, + const GraphicHwOps *hw_ops, + void *opaque) +{ +con->hw_ops = hw_ops; +con->hw = opaque; +} + QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head, const GraphicHwOps *hw_ops, void *opaque) @@ -1691,8 +1699,7 @@ QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head, ds = get_alloc_displaystate(); trace_console_gfx_new(); s = new_console(ds, GRAPHIC_CONSOLE, head); -s->hw_ops = hw_ops; -s->hw = opaque; +graphic_console_set_hwops(s, hw_ops, opaque); if (dev) { object_property_set_link(OBJECT(s), OBJECT(dev), "device", &error_abort); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API
On Mon, 29 Sep 2014 13:16:08 +0300 "Michael S. Tsirkin" wrote: > On Fri, Sep 26, 2014 at 09:28:05AM +, Igor Mammedov wrote: > > Changes since v1: > > * added usb-uas test > > * drop hotplug check in bus_add_child() > > * make SCSI & USB bus as default HotplugHandler > > * fixed dummy HBA hot(un)plug > > * fixed hotunplug on s390x > > * prevent hotplug of non hotpluggable devices > > For pci, virtio, acpi bits: > > Reviewed-by: Michael S. Tsirkin > > Which tree should this go in through? afaerber's? > In that case, it might be a good idea to Cc him ... I'm not sure through which tree it should go, it's not QOM or CPU that's why Andreas wasn't even CCed. Could you take it through your tree? > > > -- > > > > Series unifies different hotplug mechanisms to a recent > > hotplug-handler API and does shallow conversion of > > devices that still use legacy qdev hotplug to it dropping > > not used after that legacy hotplug path [29/30]. > > It also relaces SCSI's own way to do hotplug/unplug with > > hotplug-handler callbacks leaving it the only method > > perform hotplug tasks. > > And the last patch [30/30] allows to unplug of BUS-less > > devices using hotplug-handler API. > > > > Converted devices are covered with new hotplug > > unit-tests, except of: > > s390x-*: I have no idea how or means to test it, but code > > is close to virtio, so it's converted using > > virtio template > > pvscsi: is broken, so no means to test it > > > > Git tree for testing: > > https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2 > > > > > > Igor Mammedov (36): > > test: virtio-scsi: check if hot-plug/unplug works > > test: virtio-serial: check if hot-plug/unplug works > > test: libqos: add qpci_plug_device_test() and > > qpci_unplug_acpi_device_test() > > test: virtio-rng: check if hot-plug/unplug works > > test: virtio-net: check if hot-plug/unplug works > > test: virtio-blk: check if hot-plug/unplug works > > test: usb: move uhci port test code to libqos/usb.c > > test: usb: add port test to uhci unit test > > test: usb: generic usb device hotplug > > test: usb: usb-storage hotplug test > > test: usb: usb-uas hotplug test > > access BusState.allow_hotplug using wraper qbus_is_hotpluggable() > > qdev: do not allow to instantiate non hotpluggable device with > > device_add > > qdev: HotplugHandler: rename unplug callback to unplug_request > > qdev: HotplugHandler: provide unplug callback > > qdev: add simple/generic unplug callback for HotplugHandler > > qdev: add wrapper to set BUS as HotplugHandler > > qdev: drop hotplug check from bus_add_child() > > target-i386: ICC bus: drop BusState.allow_hotplug > > virtio-pci: drop BusState.allow_hotplug > > virtio-serial: convert to hotplug-handler API > > virtio-mmio: drop useless bus->allow_hotplug = 0 > > s390x: drop not used allow_hotplug in event-facility > > s390x: convert s390-virtio to hotplug handler API > > s390x: convert virtio-ccw to hotplug handler API > > scsi: set SCSI BUS itself as default HotplugHandler > > scsi: convert pvscsi HBA to hotplug handler API > > scsi: convert virtio-scsi HBA to hotplug handler API > > scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} > > fields usb-bot: mark device as non hotpluggable > > usb-bot: drop not needed "allow_hotplug = 0" > > usb-storage: drop not needed "allow_hotplug = 0" > > usb: convert usb-ccid to hotplug handler API > > usb: convert usb devices to hotplug handler API > > qdev: drop legacy hotplug fields/methods > > qdev: HotplugHandler: add support for unplugging BUS-less devices > > > > hw/acpi/piix4.c| 6 +-- > > hw/char/virtio-serial-bus.c| 20 +++--- > > hw/core/hotplug.c | 11 ++ > > hw/core/qdev.c | 85 > > -- > > hw/cpu/icc_bus.c | 8 > > hw/i386/acpi-build.c | 2 +- > > hw/isa/lpc_ich9.c | 6 +-- > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > hw/pci/pci-hotplug-old.c | 4 +- > > hw/pci/pcie.c | 4 +- > > hw/pci/pcie_port.c | 2 +- > > hw/pci/shpc.c | 4 +- > > hw/s390x/event-facility.c | 2 - > > hw/s390x/s390-virtio-bus.c | 12 +++--- > > hw/s390x/virtio-ccw.c | 17 + > > hw/scsi/scsi-bus.c | 24 > > hw/scsi/virtio-scsi.c | 30 ++- > > hw/scsi/vmw_pvscsi.c | 26 + > > hw/usb/bus.c | 9 - > > hw/usb/dev-smartcard-reader.c | 8 +++- > > hw/usb/dev-storage.c | 4 +- > > hw/virtio/virtio-mmio.c| 17 + > > hw/virtio/virtio-pci.c | 3 -- > > include/hw/hotplug.h | 16 +++- > > include/hw/pci/pcie.h | 4 +- > > include/hw/pci/shpc.h | 4 +- > > include/hw/qdev-core.h | 17 + > > include/hw/scsi/
Re: [Qemu-devel] [PATCH] main-loop: Use epoll on Linux
On Mon, Sep 29, 2014 at 01:26:29PM +0800, Fam Zheng wrote: > +int qemu_epoll(GPollFD *fds, guint nfds, int64_t timeout) > +{ > +/* A copy of last fd array, used to skip epoll_prepare when nothing > + * changed. */ > +static GPollFD *last_fds; > +static guint last_nfds; > +/* An array of fds that failed epoll_ctl and fall back to ppoll. Rare > case > + * too. */ > +static GPollFD *g_poll_fds; > +static guint g_poll_nfds; > +static int *g_poll_fd_idx; > +static int epollfd = -1; These static variables will suffer from race conditions since IOThread AioContext also calls qemu_poll_ns() when dataplane is active. Stefan pgpng4h2HLULw.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
On Mon, 29 Sep 2014, Stefano Stabellini wrote: > On Fri, 26 Sep 2014, Don Slutz wrote: > > This adds synchronisation of the vcpu registers > > between Xen and QEMU. > > > > Signed-off-by: Don Slutz > > [...] > > > diff --git a/xen-hvm.c b/xen-hvm.c > > index 05e522c..e1274bb 100644 > > --- a/xen-hvm.c > > +++ b/xen-hvm.c > > @@ -857,14 +857,48 @@ static void cpu_handle_ioreq(void *opaque) > > > > handle_buffered_iopage(state); > > if (req) { > > +#ifdef IOREQ_TYPE_VMWARE_PORT > > Is there any reason to #ifdef this code? > Couldn't we just always build it in QEMU? > > > > +if (req->type == IOREQ_TYPE_VMWARE_PORT) { > > I think it would make more sense to check for IOREQ_TYPE_VMWARE_PORT > from within handle_ioreq. > > > > +CPUX86State *env; > > +ioreq_t fake_req = { > > +.type = IOREQ_TYPE_PIO, > > +.addr = (uint16_t)req->size, > > +.size = 4, > > +.dir = IOREQ_READ, > > +.df = 0, > > +.data_is_ptr = 0, > > +}; Why do you need a fake req? Couldn't Xen send the real req instead? If any case you should spend a few more words on why you are doing this. > > +if (!xen_opaque_env) { > > +xen_opaque_env = g_malloc(sizeof(CPUX86State)); > > +} > > +env = xen_opaque_env; > > +env->regs[R_EAX] = (uint32_t)(req->addr >> 32); > > +env->regs[R_EBX] = (uint32_t)(req->addr); > > +env->regs[R_ECX] = req->count; > > +env->regs[R_EDX] = req->size; > > +env->regs[R_ESI] = (uint32_t)(req->data >> 32); > > +env->regs[R_EDI] = (uint32_t)(req->data); > > +handle_ioreq(&fake_req); > > +req->addr = ((uint64_t)fake_req.data << 32) | > > +(uint32_t)env->regs[R_EBX]; > > +req->count = env->regs[R_ECX]; > > +req->size = env->regs[R_EDX]; > > +req->data = ((uint64_t)env->regs[R_ESI] << 32) | > > +(uint32_t)env->regs[R_EDI]; > > This code could be moved to a separate helper function called by > handle_ioreq. > > > > +} else { > > +handle_ioreq(req); > > +} > > +#else > > handle_ioreq(req); > > +#endif >
Re: [Qemu-devel] [PATCH] main-loop: Use epoll on Linux
On Mon, Sep 29, 2014 at 01:26:29PM +0800, Fam Zheng wrote: > +static bool g_poll_fds_changed(const GPollFD *fds_a, const guint nfds_a, > + const GPollFD *fds_b, const guint nfds_b) ... > +static inline int g_io_condition_from_epoll_events(int e) > +{ Please don't use the g_*() namespace. It belongs to glib and names could collide. Stefan pgpc9QlRGoT28.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v7 00/11] target-arm: Parts of the AArch64 EL2/3 exception model
On 2014-09-26 17:23, Peter Maydell wrote: > On 26 September 2014 09:08, Edgar E. Iglesias > wrote: >> From: "Edgar E. Iglesias" >> >> Hi, >> >> This is a second round of AArch64 EL2/3 patches working on the exception >> model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and >> Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal >> delivery method. >> >> This conflicts slightly with the PSCI emulation patches that Rob posted. >> A rebase should be trivial, hooking in the PSCI emulation calls in the >> HVC/SMC code. > > Thanks. I've applied these to target-arm.next, with > some minor fixups to account for the cpu-exec > refactoring. Pushed to my git repo if you want > to grab it before I get round to doing a pullreq: > git://git.linaro.org/people/pmaydell/qemu-arm.git target-arm.next > > (I would still have preferred if we'd just implemented > the interrupt routing right to start with but in > the interests of making progress I'll let that pass.) Sorry for hijacking the thread, but it seems related: These bits address AArch64, but what is the status of AArch32 /wrt hyp mode emulation? After playing with the "fast" model, I would be glad to find such support in QEMU rather sooner than later. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API
On Mon, Sep 29, 2014 at 12:28:04PM +0200, Igor Mammedov wrote: > On Mon, 29 Sep 2014 13:16:08 +0300 > "Michael S. Tsirkin" wrote: > > > On Fri, Sep 26, 2014 at 09:28:05AM +, Igor Mammedov wrote: > > > Changes since v1: > > > * added usb-uas test > > > * drop hotplug check in bus_add_child() > > > * make SCSI & USB bus as default HotplugHandler > > > * fixed dummy HBA hot(un)plug > > > * fixed hotunplug on s390x > > > * prevent hotplug of non hotpluggable devices > > > > For pci, virtio, acpi bits: > > > > Reviewed-by: Michael S. Tsirkin > > > > Which tree should this go in through? afaerber's? > > In that case, it might be a good idea to Cc him ... > I'm not sure through which tree it should go, > it's not QOM or CPU that's why Andreas wasn't even CCed. > > Could you take it through your tree? As usual with cross-tree changes, selecting a tree is not obvious, but Andreas was handling most core/qdev patches. So sure, I can merge these if I get his ack. > > > > > -- > > > > > > Series unifies different hotplug mechanisms to a recent > > > hotplug-handler API and does shallow conversion of > > > devices that still use legacy qdev hotplug to it dropping > > > not used after that legacy hotplug path [29/30]. > > > It also relaces SCSI's own way to do hotplug/unplug with > > > hotplug-handler callbacks leaving it the only method > > > perform hotplug tasks. > > > And the last patch [30/30] allows to unplug of BUS-less > > > devices using hotplug-handler API. > > > > > > Converted devices are covered with new hotplug > > > unit-tests, except of: > > > s390x-*: I have no idea how or means to test it, but code > > > is close to virtio, so it's converted using > > > virtio template > > > pvscsi: is broken, so no means to test it > > > > > > Git tree for testing: > > > https://github.com/imammedo/qemu/commits/hp_ctrl_conversion_v2 > > > > > > > > > Igor Mammedov (36): > > > test: virtio-scsi: check if hot-plug/unplug works > > > test: virtio-serial: check if hot-plug/unplug works > > > test: libqos: add qpci_plug_device_test() and > > > qpci_unplug_acpi_device_test() > > > test: virtio-rng: check if hot-plug/unplug works > > > test: virtio-net: check if hot-plug/unplug works > > > test: virtio-blk: check if hot-plug/unplug works > > > test: usb: move uhci port test code to libqos/usb.c > > > test: usb: add port test to uhci unit test > > > test: usb: generic usb device hotplug > > > test: usb: usb-storage hotplug test > > > test: usb: usb-uas hotplug test > > > access BusState.allow_hotplug using wraper qbus_is_hotpluggable() > > > qdev: do not allow to instantiate non hotpluggable device with > > > device_add > > > qdev: HotplugHandler: rename unplug callback to unplug_request > > > qdev: HotplugHandler: provide unplug callback > > > qdev: add simple/generic unplug callback for HotplugHandler > > > qdev: add wrapper to set BUS as HotplugHandler > > > qdev: drop hotplug check from bus_add_child() > > > target-i386: ICC bus: drop BusState.allow_hotplug > > > virtio-pci: drop BusState.allow_hotplug > > > virtio-serial: convert to hotplug-handler API > > > virtio-mmio: drop useless bus->allow_hotplug = 0 > > > s390x: drop not used allow_hotplug in event-facility > > > s390x: convert s390-virtio to hotplug handler API > > > s390x: convert virtio-ccw to hotplug handler API > > > scsi: set SCSI BUS itself as default HotplugHandler > > > scsi: convert pvscsi HBA to hotplug handler API > > > scsi: convert virtio-scsi HBA to hotplug handler API > > > scsi: cleanup not used anymore SCSIBusInfo{hotplug,hot_unplug} > > > fields usb-bot: mark device as non hotpluggable > > > usb-bot: drop not needed "allow_hotplug = 0" > > > usb-storage: drop not needed "allow_hotplug = 0" > > > usb: convert usb-ccid to hotplug handler API > > > usb: convert usb devices to hotplug handler API > > > qdev: drop legacy hotplug fields/methods > > > qdev: HotplugHandler: add support for unplugging BUS-less devices > > > > > > hw/acpi/piix4.c| 6 +-- > > > hw/char/virtio-serial-bus.c| 20 +++--- > > > hw/core/hotplug.c | 11 ++ > > > hw/core/qdev.c | 85 > > > -- > > > hw/cpu/icc_bus.c | 8 > > > hw/i386/acpi-build.c | 2 +- > > > hw/isa/lpc_ich9.c | 6 +-- > > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > > hw/pci/pci-hotplug-old.c | 4 +- > > > hw/pci/pcie.c | 4 +- > > > hw/pci/pcie_port.c | 2 +- > > > hw/pci/shpc.c | 4 +- > > > hw/s390x/event-facility.c | 2 - > > > hw/s390x/s390-virtio-bus.c | 12 +++--- > > > hw/s390x/virtio-ccw.c | 17 + > > > hw/scsi/scsi-bus.c | 24 > > > hw/scsi/virtio-scsi.c | 30 ++- > > > hw/scsi/vmw_pvscsi.c | 26 +--
Re: [Qemu-devel] [PATCH v7 00/11] target-arm: Parts of the AArch64 EL2/3 exception model
On 29 September 2014 11:31, Jan Kiszka wrote: > Sorry for hijacking the thread, but it seems related: These bits address > AArch64, but what is the status of AArch32 /wrt hyp mode emulation? > After playing with the "fast" model, I would be glad to find such > support in QEMU rather sooner than later. I don't think anybody is currently working on that, though it is an obvious gap to fill in at some point. I would caution against testing hypervisors on QEMU rather than against hardware or the Fast Model, except for anything other than basic smoke tests, though -- there are a lot of complications and corner cases in the architecture here and I expect we will spend some time flushing out bugs as OSes expose them. -- PMM
Re: [Qemu-devel] [PULL 00/12] Trivial patches for 2014-09-26
On 26 September 2014 18:36, Michael Tokarev wrote: > Here goes another trivial-patches pull request. Cleanups, > travis.yml changes, coding style changes and a little > bugfix for stable (avoiding running qom test twice). > > Please consider applying/pulling. > > Thanks, > > /mjt > > The following changes since commit da1c4ec88ad50c2b73d5fe960c373693f7337cc9: > > Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' > into staging (2014-09-26 12:26:07 +0100) > > are available in the git repository at: > > git://git.corpit.ru/qemu.git tags/trivial-patches-2014-09-26 > > for you to fetch changes up to e5048d15ce6addae869f23514b2a1f0d4466418a: > > os-posix: report error message when lock file failed (2014-09-26 21:21:09 > +0400) > > > trivial patches for 2014-09-26 > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v7 00/11] target-arm: Parts of the AArch64 EL2/3 exception model
On 2014-09-29 12:41, Peter Maydell wrote: > On 29 September 2014 11:31, Jan Kiszka wrote: >> Sorry for hijacking the thread, but it seems related: These bits address >> AArch64, but what is the status of AArch32 /wrt hyp mode emulation? >> After playing with the "fast" model, I would be glad to find such >> support in QEMU rather sooner than later. > > I don't think anybody is currently working on that, > though it is an obvious gap to fill in at some point. > > I would caution against testing hypervisors on QEMU > rather than against hardware or the Fast Model, > except for anything other than basic smoke tests, > though -- there are a lot of complications and corner > cases in the architecture here and I expect we will > spend some time flushing out bugs as OSes expose > them. No difference with nested virt for KVM: I fixed several corner cases along the way of getting Jailhouse running on x86. This time we already have a working baseline, so shaking out bugs might be even easier. What matters more mid- and long-term is the round-trip time, in fact. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v3 0/3] add check for PCIe root ports and downstream ports
Hi, Michael Would you give me some response? I have pinged this patch series the third time. :( Thanks a lot! Best regards, -Gonglei > -Original Message- > From: Gonglei (Arei) > Sent: Monday, September 01, 2014 9:29 PM > To: qemu-devel@nongnu.org > Cc: m...@redhat.com; Huangweidong (C); pbonz...@redhat.com; > afaer...@suse.de; imamm...@redhat.com; peter.crosthwa...@xilinx.com; > Huangpeng (Peter); arm...@redhat.com; marce...@redhat.com; Luonengjun; > Gonglei (Arei) > Subject: [PATCH v3 0/3] add check for PCIe root ports and downstream ports > Importance: High > > From: Gonglei > > Root ports and downstream ports of switches are the hot > pluggable ports in a PCI Express hierarchy. > PCI Express supports chip-to-chip interconnect, a PCIe link can > only connect one pci device/Switch/EndPoint or PCI-bridge. > > 7.3. Configuration Transaction Rules (PCI Express specification 3.0) > 7.3.1. Device Number > > Downstream Ports that do not have ARI Forwarding enabled must > associate only Device 0 with the device attached to the Logical Bus > representing the Link from the Port. > > In QEMU, ARI Forwarding is enabled defualt at emulation of PCIe > ports. ARI Forwarding enable setting at firmware/OS Control handoff. > If the bit is Set when a non-ARI Device is present, the non-ARI > Device can respond to Configuration Space accesses under what it > interprets as being different Device Numbers, and its Functions can > be aliased under multiple Device Numbers, generally leading to > undesired behavior. > > So, for pci devices attached in pcie root ports or downstream pots, > we shoud assure that its slot is non-zero. For pcie devcies, which > ARP capbility is not enabled, we also should assure that its slot > is non-zero. > > Changes since v2: > - make patch 1/3 more simpler and safer.(Hu Tao) > - change check logic from pci.c to pcie.c and change function's name > - judge devcies' ARI capbility instead of PCIe ports' ARI Forwarding >(Michael) > - add trivial patch 3/3 > - update patch's commit messages and code comments. > > Thanks for your reviewing. > > Changes since v1: > - using object_dynamic_cast() instead of simple string comparing (Paolo) > - add ARI Forwarding enable bit check > - using pcie_cap_get_type() instead of simple string comparing (Marcel) > - fix some other comments. > > Gonglei (3): > qdev: Introduce a function to get qbus's parent > pcie: add check for ari capability of pcie devices > pcie: remove confused comments > > hw/core/qdev.c | 9 > hw/pci/pci.c | 4 > hw/pci/pcie.c | 59 > +++--- > include/hw/pci/pcie.h | 1 + > include/hw/qdev-core.h | 1 + > 5 files changed, 66 insertions(+), 8 deletions(-) > > -- > 1.7.12.4 >
Re: [Qemu-devel] [PATCH v4 7/7] virtio-scsi: Handle TMF request cancellation asynchronously
Il 29/09/2014 12:11, Paolo Bonzini ha scritto: > Il 28/09/2014 03:48, Fam Zheng ha scritto: >> +virtio_scsi_complete_req(req); >> +} else { >> +assert(r = -EINPROGRESS); >> +} >> } > > = instead of == here. > > Apart from this, the patch looks good. Thanks! Hmm, there's actually another problem. I think you cannot be sure that the two visits of d->requests see the same elements. In particular, one request could have non-NULL r->hba_private in the first loop, but it could become NULL by the time you reach the second loop. So we either use one loop only, or we have to save the list of requests in a separate list (for example a GSList). We can use a single loop by adding 1 to the "remaining" count while virtio_scsi_do_tmf is running, and dropping it at the end. However, you then add unnecessary complication to check for QUERY_TASK_SET around the allocation of the VirtIOSCSICancelTracker, and to free the VirtIOSCSICancelTracker if the TMF actually had nothing to do. If we move the "remaining" field inside VirtIOSCSIReq, things become much simpler. Can you review this incremental patch on top of yours? diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 7a6b71a..9e56528 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -209,13 +209,8 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) } typedef struct { +Notifiernotifier; VirtIOSCSIReq *tmf_req; -intremaining; -} VirtIOSCSICancelTracker; - -typedef struct { -Notifier notifier; -VirtIOSCSICancelTracker *tracker; } VirtIOSCSICancelNotifier; static void virtio_scsi_cancel_notify(Notifier *notifier, void *data) @@ -224,9 +219,8 @@ static void virtio_scsi_cancel_notify(Notifier *notifier, void *data) VirtIOSCSICancelNotifier, notifier); -if (--n->tracker->remaining == 0) { -virtio_scsi_complete_req(n->tracker->tmf_req); -g_slice_free(VirtIOSCSICancelTracker, n->tracker); +if (--n->tmf_req->remaining == 0) { +virtio_scsi_complete_req(n->tmf_req); } g_slice_free(VirtIOSCSICancelNotifier, n); } @@ -241,7 +235,6 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) BusChild *kid; int target; int ret = 0; -int cancel_count; if (s->dataplane_started && bdrv_get_aio_context(d->conf.bs) != s->ctx) { aio_context_acquire(s->ctx); @@ -280,15 +273,10 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; } else { VirtIOSCSICancelNotifier *notifier; -VirtIOSCSICancelTracker *tracker; - -notifier = g_slice_new(VirtIOSCSICancelNotifier); -notifier->notifier.notify - = virtio_scsi_cancel_notify; -tracker= g_slice_new(VirtIOSCSICancelTracker); -tracker->tmf_req = req; -tracker->remaining = 1; -notifier->tracker = tracker; + +req->remaining = 1; +notifier = g_slice_new(VirtIOSCSICancelNotifier); +notifier->notifier.notify = virtio_scsi_cancel_notify; scsi_req_cancel_async(r, ¬ifier->notifier); ret = -EINPROGRESS; } @@ -316,7 +304,13 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) { goto incorrect_lun; } -cancel_count = 0; + +/* Add 1 to "remaining" until virtio_scsi_do_tmf returns. + * This way, if the bus starts calling back to the notifiers + * even before we finish the loop, virtio_scsi_cancel_notify + * will not complete the TMF too early. + */ +req->remaining = 1; QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) { if (r->hba_private) { if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) { @@ -324,35 +318,19 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED; break; } else { -/* Before we actually cancel any requests in the next for - * loop, let's count them. This way, if the bus starts - * calling back to the notifier even before we finish the - * loop, the counter, which value is already seen in - * virtio_scsi_cancel_notify, will prevent us from - * completing the tmf too quickly. */ -cancel_count++; -} -} -} -if (cancel_count) { -
Re: [Qemu-devel] [PATCH v2 17/36] qdev: add wrapper to set BUS as HotplugHandler
Il 26/09/2014 11:28, Igor Mammedov ha scritto: > to be used for conversion of SCSI and USB devices, > and would allow to make every HBA/USB host switch > to HotplugHandler API without touching each controller > explicitly. > > Signed-off-by: Igor Mammedov > --- > hw/core/qdev.c | 19 +++ > include/hw/qdev-core.h | 11 --- > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 1d1b113..0de99b2 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -112,6 +112,25 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) > bus_add_child(bus, dev); > } > > +static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler, > + Error **errp) > +{ > + > +object_property_set_link(OBJECT(bus), OBJECT(handler), > + QDEV_HOTPLUG_HANDLER_PROPERTY, errp); > +bus->allow_hotplug = 1; > +} > + > +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error > **errp) > +{ > +qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp); > +} > + > +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp) > +{ > +qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp); > +} > + > /* Create a new device. This only initializes the device state structure > and allows properties to be set. qdev_init should be called to > initialize the actual device emulation. */ > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index ba812c5..48e9579 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -363,13 +363,10 @@ extern int qdev_hotplug; > > char *qdev_get_dev_path(DeviceState *dev); > > -static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState > *handler, > -Error **errp) > -{ > -object_property_set_link(OBJECT(bus), OBJECT(handler), > - QDEV_HOTPLUG_HANDLER_PROPERTY, errp); > -bus->allow_hotplug = 1; > -} > +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, > + Error **errp); > + > +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp); > > static inline bool qbus_is_hotpluggable(BusState *bus) > { > Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH v2 13/36] qdev: do not allow to instantiate non hotpluggable device with device_add
Il 26/09/2014 11:28, Igor Mammedov ha scritto: > It will allow explicitly mark device as not hotpluggable and > avoid its creation with following error at realize time > and destroying it afterwards anyway. Instead of it will > error out even before instance of device is created. > > Signed-off-by: Igor Mammedov > --- > qdev-monitor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/qdev-monitor.c b/qdev-monitor.c > index f6db461..c721451 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -487,7 +487,8 @@ DeviceState *qdev_device_add(QemuOpts *opts) > } > > dc = DEVICE_CLASS(oc); > -if (dc->cannot_instantiate_with_device_add_yet) { > +if (dc->cannot_instantiate_with_device_add_yet || > +(qdev_hotplug && !dc->hotpluggable)) { > qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", >"pluggable device type"); > return NULL; > Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH v2 19/36] target-i386: ICC bus: drop BusState.allow_hotplug
Il 26/09/2014 11:28, Igor Mammedov ha scritto: > since bus_add_child() no longer cares if BUS is hotpluggable > or not, there is no need in setting allow_hotplug field. > > Signed-off-by: Igor Mammedov > --- > hw/cpu/icc_bus.c | 8 > 1 file changed, 8 deletions(-) > > diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c > index 7f44c59..9575fd6 100644 > --- a/hw/cpu/icc_bus.c > +++ b/hw/cpu/icc_bus.c > @@ -24,18 +24,10 @@ > > /* icc-bridge implementation */ > > -static void icc_bus_init(Object *obj) > -{ > -BusState *b = BUS(obj); > - > -b->allow_hotplug = true; > -} > - > static const TypeInfo icc_bus_info = { > .name = TYPE_ICC_BUS, > .parent = TYPE_BUS, > .instance_size = sizeof(ICCBus), > -.instance_init = icc_bus_init, > }; > > > Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH v2 18/36] qdev: drop hotplug check from bus_add_child()
Il 26/09/2014 11:28, Igor Mammedov ha scritto: > check is too restrictive and does not allow > to add childs to just created bus during hotplug > when the bus is part of composite device. > > Removing check from bus_add_child() doesn't affect > devices creatable with device_add/del commands since > they have a similar builtin checks and patch will > allow to create complex composite devices during > hotplug. > > Signed-off-by: Igor Mammedov > --- > hw/core/qdev.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 0de99b2..fa86843 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -85,10 +85,6 @@ static void bus_add_child(BusState *bus, DeviceState > *child) > char name[32]; > BusChild *kid = g_malloc0(sizeof(*kid)); > > -if (qdev_hotplug) { > -assert(qbus_is_hotpluggable(bus)); > -} > - > kid->index = bus->max_index++; > kid->child = child; > object_ref(OBJECT(kid->child)); > Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
On Sat, 27 Sep 2014 10:37:23 + "Gonglei (Arei)" wrote: > > > One thing I noticed is that the various devices end up with similar > > > code in the end: > > > > > > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER); > > > object_property_add_child(obj, "virtio-backend", > > OBJECT(&dev->vdev), > > > NULL); > > > object_unref(OBJECT(&dev->vdev)); > > > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > > > > > > Would it make sense to add a helper function for that? > > > > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different > for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock), > VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters > for above code segment. :) Hm... void virtio_instance_init_common(Object *proxydev, DeviceState *vdev, size_t vdevsize, const char *vdevname) { object_initialize(vdev, vdevsize, vdevname); object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev), NULL); object_unref(OBJECT(vdev)); qdev_alias_all_properties(vdev, proxydev); } and have the initializers call virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev), TYPE_WHATEVER); ?
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
Il 29/09/2014 10:12, Alexander Graf ha scritto: > Could you instead plug into the existing cpu_synchronize_registers() > framework and just implement register synchronization for the Xen side, > just like it's been done for KVM? :) No, because here it's Xen that sends out the register contents. With KVM, it's QEMU that requests the register contents. Paolo
Re: [Qemu-devel] [PATCH v2 14/36] qdev: HotplugHandler: rename unplug callback to unplug_request
On Fri, Sep 26, 2014 at 09:28:19AM +, Igor Mammedov wrote: > 'HotplugHandler.unplug' callback is currently used as async > call to issue unplug request for device that implements it. > Renaming 'unplug' callback to 'unplug_request' should help to > avoid confusion about what callback does and would allow to > introduce 'unplug' callback that would perform actual device > removal when guest is ready for it. > > Signed-off-by: Igor Mammedov > Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin > --- > Patch is prompted by reviewing https://patchwork.ozlabs.org/patch/383372/ > > Dedicated 'unplug' callback could be used by bus-less pc-dimm > device. It would allow to call HotplugHandler.unplug callback > from ACPI code when guest calls _EJ0 method and execute board > specific code (PCMachine) to unmap pc-dimm from guest's address > space and perform necessary cleanup. The same applies to CPU > unplug. > --- > hw/acpi/piix4.c| 6 +++--- > hw/core/hotplug.c | 10 +- > hw/core/qdev.c | 3 ++- > hw/isa/lpc_ich9.c | 6 +++--- > hw/pci-bridge/pci_bridge_dev.c | 2 +- > hw/pci/pcie.c | 4 ++-- > hw/pci/pcie_port.c | 2 +- > hw/pci/shpc.c | 4 ++-- > include/hw/hotplug.h | 16 +--- > include/hw/pci/pcie.h | 4 ++-- > include/hw/pci/shpc.h | 4 ++-- > 11 files changed, 32 insertions(+), 29 deletions(-) > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index b72b34e..0bfa814 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -354,8 +354,8 @@ static void piix4_device_plug_cb(HotplugHandler > *hotplug_dev, > } > } > > -static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, > - DeviceState *dev, Error **errp) > +static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > { > PIIX4PMState *s = PIIX4_PM(hotplug_dev); > > @@ -615,7 +615,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void > *data) > dc->cannot_instantiate_with_device_add_yet = true; > dc->hotpluggable = false; > hc->plug = piix4_device_plug_cb; > -hc->unplug = piix4_device_unplug_cb; > +hc->unplug_request = piix4_device_unplug_request_cb; > adevc->ospm_status = piix4_ospm_status; > } > > diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c > index 5573d9d..2ec4736 100644 > --- a/hw/core/hotplug.c > +++ b/hw/core/hotplug.c > @@ -23,14 +23,14 @@ void hotplug_handler_plug(HotplugHandler *plug_handler, > } > } > > -void hotplug_handler_unplug(HotplugHandler *plug_handler, > -DeviceState *plugged_dev, > -Error **errp) > +void hotplug_handler_unplug_request(HotplugHandler *plug_handler, > +DeviceState *plugged_dev, > +Error **errp) > { > HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); > > -if (hdc->unplug) { > -hdc->unplug(plug_handler, plugged_dev, errp); > +if (hdc->unplug_request) { > +hdc->unplug_request(plug_handler, plugged_dev, errp); > } > } > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 5e5b963..c98e5db 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -227,7 +227,8 @@ void qdev_unplug(DeviceState *dev, Error **errp) > qdev_hot_removed = true; > > if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > -hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp); > +hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler, > + dev, errp); > } else { > assert(dc->unplug != NULL); > if (dc->unplug(dev) < 0) { /* legacy handler */ > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 177023b..530b074 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -607,8 +607,8 @@ static void ich9_device_plug_cb(HotplugHandler > *hotplug_dev, > ich9_pm_device_plug_cb(&lpc->pm, dev, errp); > } > > -static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev, > - DeviceState *dev, Error **errp) > +static void ich9_device_unplug_request_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > { > error_setg(errp, "acpi: device unplug request for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -676,7 +676,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void > *data) > */ > dc->cannot_instantiate_with_device_add_yet = true; > hc->plug = ich9_device_plug_cb; > -hc->unplug = ich9_device_unplug_cb; > +hc->unplug_request = ich9_device_unplug_request_cb; > adevc->ospm_status = ich9_pm
Re: [Qemu-devel] [PATCH] libqos: use microseconds instead of iterations for virtio timeout
On 26 September 2014 17:35, Stefan Hajnoczi wrote: > Some hosts are slow or overloaded so test execution takes a long time. > Test cases use timeouts to protect against an infinite loop stalling the > test forever (especially important in automated test setups). > > Commit 6cd14054b67774cc58a51fca6660cfa1d3c08059 ("libqos virtio: > Increase ISR timeout") increased the clock_step() value in an attempt to > lengthen the virtio interrupt wait timeout, but timeout failures are > still occuring on the Travis automated testing platform. > > This is because clock_step() only affects the guest's virtual time. > Virtio requests can be bottlenecked on host disk I/O latency - which > cannot be improved by stepping the clock, so the fix was ineffective. > > This patch changes the qvirtio_wait_queue_isr() and > qvirtio_wait_config_isr() timeout mechanism from loop iterations to > microseconds. This way the test case can specify an absolute 30 second > timeout. Number of loop iterations is not a reliable timeout mechanism > since the speed depends on many factors including host performance. > > Tests should no longer timeout on overloaded Travis instances. > > Note that I dropped an assertion that waits for the full timeout > duration to assert that no interrupt was raised. I think this is not a > huge loss because it makes the test case non-deterministic (it cannot > really prove the no interrupt will ever be raised). As per IRC conversation, this patch causes 'make check' to fail on some (slower?) systems with: ERROR:/home/petmay01/linaro/qemu-for-merges/tests/virtio-blk-test.c:596:pci_idx: assertion failed (status == 0): (255 == 0) Stefan has diagnosed this as a failure to poll for the zero status code, and is working on a patch. -- PMM
Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
On Fri, 26 Sep 2014 16:45:38 +0800 wrote: > Gonglei (9): > virtio-net: use aliases instead of duplicate qdev properties > virtio: fix virtio-net child refcount in transports > virtio/vhost scsi: use aliases instead of duplicate qdev properties > virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in > transports > virtio-serial: use aliases instead of duplicate qdev properties > virtio-serial: fix virtio-serial child refcount in transports > virtio-rng: use aliases instead of duplicate qdev properties > virtio-rng: fix virtio-rng child refcount in transports > virtio-balloon: fix virtio-balloon child refcount in transports > > hw/s390x/s390-virtio-bus.c | 16 ++-- > hw/s390x/virtio-ccw.c | 18 +++--- > hw/virtio/virtio-pci.c | 18 +++--- > 3 files changed, 32 insertions(+), 20 deletions(-) > The patchset passes a small snifftest for s390-virtio and virtio-ccw: boots, device_add and device_del work as expected.
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
On 29.09.14 13:10, Paolo Bonzini wrote: > Il 29/09/2014 10:12, Alexander Graf ha scritto: >> Could you instead plug into the existing cpu_synchronize_registers() >> framework and just implement register synchronization for the Xen side, >> just like it's been done for KVM? :) > > No, because here it's Xen that sends out the register contents. With > KVM, it's QEMU that requests the register contents. So? We could still reuse the same infrastructure: cpu_handle_ioreq() { ... if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { cpu->xen_vcpu_dirty = true; synchronize_xen_to_env(xenptr, cpu); } handle_ioreq(); if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { cpu->xen_vcpu_dirty = false; synchronize_env_to_xen(xenptr, cpu); } ... } void xen_cpu_synchronize_state(CPUState *cpu) { assert(cpu->xen_vcpu_dirty); } Then no changes to the vmport code would be necessary and this problems where some code path wants to do direct access to registers automatically tells us that things are broken. Alex
Re: [Qemu-devel] KVM call for agenda for 2014-09-30
On 28/09/2014 18:33, Juan Quintela wrote: Hi Please, send any topic that you are interested in covering. Call details: 15:00 CEST 13:00 UTC 09:00 EDT Every two weeks By popular demand, a google calendar public entry with it https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ (Let me know if you have any problems with the calendar entry) If you need phone number details, contact me privately Thanks, Juan. Hi Juan, All, We wanted to add two topics.. The first about reverse execution: Paolo suggested we have a talk about this before the KVM forum including Pavel to compair both approaches. The second is about TCG multi thread implementation in QEMU. Thanks, Fred
Re: [Qemu-devel] [PATCH v3 15/23] ide: Complete conversion from BlockDriverState to BlockBackend
Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: > Add a BlockBackend member to TrimAIOCB, so ide_issue_trim_cb() can use > blk_aio_discard() instead of bdrv_aio_discard(). > > Signed-off-by: Markus Armbruster Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH v3 16/23] pc87312: Drop unused members of PC87312State
Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: > Signed-off-by: Markus Armbruster Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> From: Cornelia Huck [mailto:cornelia.h...@de.ibm.com] > Sent: Monday, September 29, 2014 6:53 PM > Subject: Re: [PATCH RESEND 0/9] virtio: fix virtio child recount in transports > > On Sat, 27 Sep 2014 10:37:23 + > "Gonglei (Arei)" wrote: > > > > > One thing I noticed is that the various devices end up with similar > > > > code in the end: > > > > > > > > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER); > > > > object_property_add_child(obj, "virtio-backend", > > > OBJECT(&dev->vdev), > > > > NULL); > > > > object_unref(OBJECT(&dev->vdev)); > > > > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > > > > > > > > Would it make sense to add a helper function for that? > > > > > > > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is > > different > > for different virtio devices, like VirtIOBlkPCI(and its vdev is > > VirtIOBlock), > > VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters > > for above code segment. :) > > Hm... > > void virtio_instance_init_common(Object *proxydev, > DeviceState *vdev, > size_t vdevsize, > const char *vdevname) > { > object_initialize(vdev, vdevsize, vdevname); > object_property_add_child(proxydev, "virtio-backend", OBJECT(vdev), > NULL); > object_unref(OBJECT(vdev)); > qdev_alias_all_properties(vdev, proxydev); > } > > and have the initializers call > > virtio_instance_init_common(obj, DEVICE(&dev->vdev), sizeof(dev->vdev), > TYPE_WHATEVER); > > ? OK, it looks good that pass all parameters to one wrapper function. Will do this in next version. Thanks, Cornelia. :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v3 17/23] blockdev: Drop superfluous DriveInfo member id
Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: > Signed-off-by: Markus Armbruster Reviewed-by: Kevin Wolf
[Qemu-devel] [PATCH V4 5/8] pc: Update rtc_cmos in pc_cpu_plug
Update rtc_cmos in pc_cpu_plug directly instead of the notifier. v4: -Make link property in PCMachine rather than the global variables. -Split out the removal of unused notifier into separate patch. Signed-off-by: Gu Zheng --- hw/i386/pc.c | 37 - hw/i386/pc_piix.c|2 +- hw/i386/pc_q35.c |2 +- include/hw/i386/pc.h |3 ++- qom/cpu.c|1 - 5 files changed, 20 insertions(+), 25 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index dcb9332..301e704 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -355,30 +355,15 @@ static void pc_cmos_init_late(void *opaque) qemu_unregister_reset(pc_cmos_init_late, opaque); } -typedef struct RTCCPUHotplugArg { -Notifier cpu_added_notifier; -ISADevice *rtc_state; -} RTCCPUHotplugArg; - -static void rtc_notify_cpu_added(Notifier *notifier, void *data) -{ -RTCCPUHotplugArg *arg = container_of(notifier, RTCCPUHotplugArg, - cpu_added_notifier); -ISADevice *s = arg->rtc_state; - -/* increment the number of CPUs */ -rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1); -} - void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, - const char *boot_device, + const char *boot_device, MachineState *machine, ISADevice *floppy, BusState *idebus0, BusState *idebus1, ISADevice *s) { int val, nb, i; FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; static pc_cmos_init_late_arg arg; -static RTCCPUHotplugArg cpu_hotplug_cb; +PCMachineState *pc_machine = PC_MACHINE(machine); /* various important CMOS locations needed by PC/Bochs bios */ @@ -417,10 +402,14 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, /* set the number of CPU */ rtc_set_memory(s, 0x5f, smp_cpus - 1); -/* init CPU hotplug notifier */ -cpu_hotplug_cb.rtc_state = s; -cpu_hotplug_cb.cpu_added_notifier.notify = rtc_notify_cpu_added; -qemu_register_cpu_added_notifier(&cpu_hotplug_cb.cpu_added_notifier); + +object_property_add_link(OBJECT(machine), "rtc_state", + TYPE_ISA_DEVICE, + (Object **)&pc_machine->rtc, + object_property_allow_set_link, + OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort); +object_property_set_link(OBJECT(machine), OBJECT(s), + "rtc_state", &error_abort); if (set_boot_dev(s, boot_device)) { exit(1); @@ -1633,6 +1622,12 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); +if (local_err) { +goto out; +} + +/* increment the number of CPUs */ +rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); out: error_propagate(errp, local_err); } diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 103d756..2c8d4dc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -266,7 +266,7 @@ static void pc_init1(MachineState *machine, } pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order, - floppy, idebus[0], idebus[1], rtc_state); + machine, floppy, idebus[0], idebus[1], rtc_state); if (pci_enabled && usb_enabled(false)) { pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index d4a907c..94ba98d 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -266,7 +266,7 @@ static void pc_q35_init(MachineState *machine) 8, NULL, 0); pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order, - floppy, idebus[0], idebus[1], rtc_state); + machine, floppy, idebus[0], idebus[1], rtc_state); /* the rest devices to which pci devfn is automatically assigned */ pc_vga_init(isa_bus, host_bus); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 77316d5..7a4bff4 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -33,6 +33,7 @@ struct PCMachineState { MemoryRegion hotplug_memory; HotplugHandler *acpi_dev; +ISADevice *rtc; uint64_t max_ram_below_4g; }; @@ -210,7 +211,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, uint32 hpet_irqs); void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd); void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, - const char *boot_device, + const char *boot_device, MachineState *machine, ISADevice *floppy, BusState *ide0, BusState *ide1, ISADevice *s); void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus); diff --git a/qom/cpu.c b/qom/cpu.c index ba8b40
[Qemu-devel] [PATCH V4 1/8] acpi/cpu: add cpu hotplug callback function to match hotplug_handler API
v4: -convert CPUState *cpu to DeviceState *dev like it's done for other handlers and do cast to CPU inside. v2: -add errp argument to catch error. -return error instead of aborting if cpu id is invalid. -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add. Signed-off-by: Gu Zheng --- hw/acpi/cpu_hotplug.c | 18 ++ include/hw/acpi/cpu_hotplug.h |3 +++ 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 2ad83a0..06e9c61 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -36,6 +36,24 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = { }, }; +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, DeviceState *dev, Error **errp) +{ +CPUState *cpu = CPU(dev); +CPUClass *k = CPU_GET_CLASS(cpu); +int64_t cpu_id; + +cpu_id = k->get_arch_id(cpu); +if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) { +error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id); +return; +} + +AcpiCpuHotplug_add(&ar->gpe, g, cpu); + +acpi_update_sci(ar, irq); +} + void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu) { CPUClass *k = CPU_GET_CLASS(cpu); diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index 9e5d30c..5dca8d7 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -20,6 +20,9 @@ typedef struct AcpiCpuHotplug { uint8_t sts[ACPI_GPE_PROC_LEN]; } AcpiCpuHotplug; +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, DeviceState *dev, Error **errp); + void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu); void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, -- 1.7.7
[Qemu-devel] [PATCH V4 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API
Previously we use cpu_added_notifiers to register cpu hotplug notifier callback which is not able to pass/handle errors, so we switch it to unified hotplug handler API which allows to pass errors and would allow to cancel device_add in case of error. Thanks very much for Igor's review and suggestion. v4: -split removal of CPU hotplug notifier into separate patch (Patch 6/8). Patch 1/7: -convert CPUState *cpu to DeviceState *dev like it's done for other handlers and do cast to CPU inside. Patch 5/7: -Make rtc_state as a link property in PCMachine rather than the global variables. -Split out the removal of unused notifier into separate patch. -Check the result of plug callback before update rtc_state. v3: -deal with start-up cpus in pc_cpu_plug as Igor suggested. v2: -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5. 1/5-->1/7 2/5-->2/7 3/5-->3/7 4/5-->4/7 Patch 1/7: -add errp argument to catch error. -return error instead of aborting if cpu id is invalid. -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add. Patch 3/7: -remove unused AcpiCpuHotplug_add directly. Patch 5/7: -switch the last user of cpu hotplug notifier to hotplug handler API, and remove the unused cpu hotplug notify. Patch 6/7: -split the function rename (just cleanup) into single patch. Patch 7/7: -introduce help function acpi_set_local_sts to keep the bit setting in one place. Gu Zheng (8): acpi/cpu: add cpu hotplug callback function to match hotplug_handler API acpi:ich9: convert cpu hotplug handle to hotplug_handler API acpi:piix4: convert cpu hotplug handle to hotplug_handler API pc: add cpu hotplug handler to PC_MACHINE pc: Update rtc_cmos in pc_cpu_plug qom/cpu: remove the unused CPU hot-plug notifier cpu-hotplug: rename function for better readability acpi/cpu-hotplug: introduce help function to keep bit setting in one place hw/acpi/cpu_hotplug.c | 35 -- hw/acpi/ich9.c| 17 ++ hw/acpi/piix4.c | 18 ++- hw/i386/pc.c | 63 ++-- hw/i386/pc_piix.c |2 +- hw/i386/pc_q35.c |2 +- include/hw/acpi/cpu_hotplug.h |7 ++-- include/hw/acpi/ich9.h|1 - include/hw/i386/pc.h |3 +- include/sysemu/sysemu.h |3 -- qom/cpu.c | 10 -- 11 files changed, 82 insertions(+), 79 deletions(-) -- 1.7.7
Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport
Il 29/09/2014 13:53, Alexander Graf ha scritto: > > cpu_handle_ioreq() > { > ... > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = true; > synchronize_xen_to_env(xenptr, cpu); > } > > handle_ioreq(); > > if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) { > cpu->xen_vcpu_dirty = false; > synchronize_env_to_xen(xenptr, cpu); > } > > ... > } > > void xen_cpu_synchronize_state(CPUState *cpu) > { > assert(cpu->xen_vcpu_dirty); > } > > Then no changes to the vmport code would be necessary and this problems > where some code path wants to do direct access to registers > automatically tells us that things are broken. Yeah, that would be possible. You do not even need synchronize_state, it seems to me that it introduces more complication for little gain. Paolo
Re: [Qemu-devel] [Xen-devel] [PATCH 3/3] xenfb: Add "feature-no-abs-rescale" for Windows PV frontend
On Thu, 2014-09-11 at 02:23 +0100, Stefano Stabellini wrote: Stefano, In the context of this patch could you also take a look at Owen's xenfb documentation patches[0], I think you probably know the most about this particular protocol. Ian. [0] <1411376699-8175-1-git-send-email-owen.sm...@citrix.com> and friends.
[Qemu-devel] [PATCH V4 2/8] acpi:ich9: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API. Signed-off-by: Gu Zheng --- hw/acpi/ich9.c | 13 ++--- include/hw/acpi/ich9.h |1 - 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 7b14bbb..7585364 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -209,15 +209,6 @@ static void pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(&pm->acpi_regs); } -static void ich9_cpu_added_req(Notifier *n, void *opaque) -{ -ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier); - -assert(pm != NULL); -AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque)); -acpi_update_sci(&pm->acpi_regs, pm->irq); -} - void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, qemu_irq sci_irq) { @@ -246,8 +237,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); -pm->cpu_added_notifier.notify = ich9_cpu_added_req; -qemu_register_cpu_added_notifier(&pm->cpu_added_notifier); if (pm->acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), @@ -304,6 +293,8 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp) object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq, &pm->gpe_cpu, dev, errp); } else { error_setg(errp, "acpi: device plug request for not supported device" " type: %s", object_get_typename(OBJECT(dev))); diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index 7e42448..fe975e6 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -47,7 +47,6 @@ typedef struct ICH9LPCPMRegs { Notifier powerdown_notifier; AcpiCpuHotplug gpe_cpu; -Notifier cpu_added_notifier; MemHotplugState acpi_memory_hotplug; } ICH9LPCPMRegs; -- 1.7.7
[Qemu-devel] [PATCH V4 3/8] acpi:piix4: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API, and remove the unused AcpiCpuHotplug_add(). v2: -remove the unused AcpiCpuHotplug_add(). Signed-off-by: Gu Zheng --- hw/acpi/cpu_hotplug.c | 14 ++ hw/acpi/piix4.c | 14 ++ include/hw/acpi/cpu_hotplug.h |2 -- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 06e9c61..b69b16c 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -49,22 +49,12 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, return; } -AcpiCpuHotplug_add(&ar->gpe, g, cpu); +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; +g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); acpi_update_sci(ar, irq); } -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu) -{ -CPUClass *k = CPU_GET_CLASS(cpu); -int64_t cpu_id; - -*gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS; -cpu_id = k->get_arch_id(CPU(cpu)); -g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN); -g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); -} - void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, AcpiCpuHotplug *gpe_cpu, uint16_t base) { diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..8320b18 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -83,7 +83,6 @@ typedef struct PIIX4PMState { uint8_t s4_val; AcpiCpuHotplug gpe_cpu; -Notifier cpu_added_notifier; MemHotplugState acpi_memory_hotplug; } PIIX4PMState; @@ -348,6 +347,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp); } else { error_setg(errp, "acpi: device plug request for not supported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -544,15 +545,6 @@ static const MemoryRegionOps piix4_gpe_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; -static void piix4_cpu_added_req(Notifier *n, void *opaque) -{ -PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier); - -assert(s != NULL); -AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque)); -acpi_update_sci(&s->ar, s->irq); -} - static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, PCIBus *bus, PIIX4PMState *s) { @@ -565,8 +557,6 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu, PIIX4_CPU_HOTPLUG_IO_BASE); -s->cpu_added_notifier.notify = piix4_cpu_added_req; -qemu_register_cpu_added_notifier(&s->cpu_added_notifier); if (s->acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug); diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index 5dca8d7..4657e71 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -23,8 +23,6 @@ typedef struct AcpiCpuHotplug { void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiCpuHotplug *g, DeviceState *dev, Error **errp); -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu); - void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, AcpiCpuHotplug *gpe_cpu, uint16_t base); #endif -- 1.7.7
[Qemu-devel] [PATCH V4 8/8] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
Introduce help function acpi_set_local_sts() to simplify acpi_cpu_plug_cb and acpi_cpu_hotplug_init, so that we can keep bit setting in one place. Signed-off-by: Gu Zheng --- hw/acpi/cpu_hotplug.c | 23 +++ 1 files changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index ae48b63..8ff8c4d 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -36,10 +36,9 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = { }, }; -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, - AcpiCpuHotplug *g, DeviceState *dev, Error **errp) +static void acpi_set_local_sts(AcpiCpuHotplug *g, CPUState *cpu, + Error **errp) { -CPUState *cpu = CPU(dev); CPUClass *k = CPU_GET_CLASS(cpu); int64_t cpu_id; @@ -49,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, return; } -ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); +} +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, DeviceState *dev, Error **errp) +{ +acpi_set_local_sts(g, CPU(dev), errp); +if (*errp != NULL) { +return; +} + +ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; acpi_update_sci(ar, irq); } @@ -61,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, CPUState *cpu; CPU_FOREACH(cpu) { -CPUClass *cc = CPU_GET_CLASS(cpu); -int64_t id = cc->get_arch_id(cpu); +Error *local_err = NULL; -g_assert((id / 8) < ACPI_GPE_PROC_LEN); -gpe_cpu->sts[id / 8] |= (1 << (id % 8)); +acpi_set_local_sts(gpe_cpu, cpu, &local_err); +g_assert(local_err == NULL); } memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops, gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN); -- 1.7.7
[Qemu-devel] [PATCH V4 7/8] cpu-hotplug: rename function for better readability
Rename: AcpiCpuHotplug_init --> acpi_cpu_hotplug_init AcpiCpuHotplug_ops --> acpi_cpu_hotplug_ops for better readability, just cleanup. Signed-off-by: Gu Zheng --- hw/acpi/cpu_hotplug.c |4 ++-- hw/acpi/ich9.c|4 ++-- hw/acpi/piix4.c |4 ++-- include/hw/acpi/cpu_hotplug.h |4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index b69b16c..ae48b63 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -55,8 +55,8 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, acpi_update_sci(ar, irq); } -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, - AcpiCpuHotplug *gpe_cpu, uint16_t base) +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, + AcpiCpuHotplug *gpe_cpu, uint16_t base) { CPUState *cpu; diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 7585364..ea991a3 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -235,8 +235,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, pm->powerdown_notifier.notify = pm_powerdown_req; qemu_register_powerdown_notifier(&pm->powerdown_notifier); -AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), -&pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); +acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), + &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); if (pm->acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 8320b18..8fde808 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -555,8 +555,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent, s->use_acpi_pci_hotplug); -AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu, -PIIX4_CPU_HOTPLUG_IO_BASE); +acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu, + PIIX4_CPU_HOTPLUG_IO_BASE); if (s->acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug); diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index 4657e71..f6d358d 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -23,6 +23,6 @@ typedef struct AcpiCpuHotplug { void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiCpuHotplug *g, DeviceState *dev, Error **errp); -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, - AcpiCpuHotplug *gpe_cpu, uint16_t base); +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, + AcpiCpuHotplug *gpe_cpu, uint16_t base); #endif -- 1.7.7
[Qemu-devel] [PATCH V4 6/8] qom/cpu: remove the unused CPU hot-plug notifier
Signed-off-by: Gu Zheng --- include/sysemu/sysemu.h |3 --- qom/cpu.c |9 - 2 files changed, 0 insertions(+), 12 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index d8539fd..acfe494 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -183,9 +183,6 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict); /* generic hotplug */ void drive_hot_add(Monitor *mon, const QDict *qdict); -/* CPU hotplug */ -void qemu_register_cpu_added_notifier(Notifier *notifier); - /* pcie aer error injection */ void pcie_aer_inject_error_print(Monitor *mon, const QObject *data); int do_pcie_aer_inject_error(Monitor *mon, diff --git a/qom/cpu.c b/qom/cpu.c index 09c7e41..83c3559 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -107,15 +107,6 @@ static void cpu_common_get_memory_mapping(CPUState *cpu, error_setg(errp, "Obtaining memory mappings is unsupported on this CPU."); } -/* CPU hot-plug notifiers */ -static NotifierList cpu_added_notifiers = -NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers); - -void qemu_register_cpu_added_notifier(Notifier *notifier) -{ -notifier_list_add(&cpu_added_notifiers, notifier); -} - void cpu_reset_interrupt(CPUState *cpu, int mask) { cpu->interrupt_request &= ~mask; -- 1.7.7
[Qemu-devel] [PATCH V4 4/8] pc: add cpu hotplug handler to PC_MACHINE
Add cpu hotplug handler to PC_MACHINE, which will perform the acpi cpu hotplug callback via hotplug_handler API. v3: -deal with start up cpus in a more neat way as Igor suggested. v2: -just rebase. Signed-off-by: Gu Zheng --- hw/i386/pc.c | 26 +- 1 files changed, 25 insertions(+), 1 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 82a7daa..dcb9332 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1616,11 +1616,34 @@ out: error_propagate(errp, local_err); } +static void pc_cpu_plug(HotplugHandler *hotplug_dev, +DeviceState *dev, Error **errp) +{ +HotplugHandlerClass *hhc; +Error *local_err = NULL; +PCMachineState *pcms = PC_MACHINE(hotplug_dev); + +if (!pcms->acpi_dev) { +if (dev->hotplugged) { +error_setg(&local_err, + "cpu hotplug is not enabled: missing acpi device"); +} +goto out; +} + +hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); +hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); +out: +error_propagate(errp, local_err); +} + static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { pc_dimm_plug(hotplug_dev, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +pc_cpu_plug(hotplug_dev, dev, errp); } } @@ -1629,7 +1652,8 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, { PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine); -if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || +object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { return HOTPLUG_HANDLER(machine); } -- 1.7.7