[Qemu-devel] [PATCH v9 0/7] monitor: let Monitor be thread safe
v9: - two more patches to implement Markus's idea to init monitor earlier (which are patch 5 & 6) - touch up patch 7 to init the fdset lock in monitor_init_globals() v8: - some wording changes according to previous comments [Markus] - return -ENOENT too in stubs/fdset.c:monitor_fdset_get_fd() [Stefan] - refactor the fdset functions a bit, drop "ret" where proper [Markus] - one more patch to fix monitor_lock comment [Markus] - regular rebase and torturing Stefan reported this problem that in the future we might start to have more threads operating on the same Monitor object. This seris try to add fundamental support for it. Please review. Thanks, Peter Xu (7): monitor: rename out_lock to mon_lock monitor: protect mon->fds with mon_lock monitor: more comments on lock-free elements monitor: fix comment for monitor_lock monitor: remove event_clock_type monitor: move init global earlier monitor: add lock to protect mon_fdsets monitor.c | 164 ++ stubs/fdset.c | 2 +- util/osdep.c | 3 +- vl.c | 7 +-- 4 files changed, 117 insertions(+), 59 deletions(-) -- 2.17.0
[Qemu-devel] [PATCH v9 3/7] monitor: more comments on lock-free elements
Add some explicit comments for both Readline and cpu_set/cpu_get helpers that they do not need the mon_lock protection. Signed-off-by: Peter Xu --- monitor.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index d6c3c08932..f23178951e 100644 --- a/monitor.c +++ b/monitor.c @@ -207,7 +207,15 @@ struct Monitor { int suspend_cnt;/* Needs to be accessed atomically */ bool skip_flush; bool use_io_thr; + +/* + * State used only in the thread "owning" the monitor. + * If @use_io_thr, this is mon_global.mon_iothread. + * Else, it's the main thread. + * These members can be safely accessed without locks. + */ ReadLineState *rs; + MonitorQMP qmp; gchar *mon_cpu_path; BlockCompletionFunc *password_completion_cb; @@ -1313,7 +1321,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable, cur_mon->qmp.commands = _commands; } -/* set the current CPU defined by the user */ +/* Set the current CPU defined by the user. Callers must hold BQL. */ int monitor_set_cpu(int cpu_index) { CPUState *cpu; @@ -1327,6 +1335,7 @@ int monitor_set_cpu(int cpu_index) return 0; } +/* Callers must hold BQL. */ static CPUState *mon_get_cpu_sync(bool synchronize) { CPUState *cpu; -- 2.17.0
[Qemu-devel] [PATCH v9 2/7] monitor: protect mon->fds with mon_lock
mon->fds were protected by BQL. Now protect it by mon_lock so that it can even be used in monitor iothread. Reviewed-by: Stefan Hajnoczi Reviewed-by: Markus Armbruster Signed-off-by: Peter Xu --- monitor.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/monitor.c b/monitor.c index 14c681dc8a..d6c3c08932 100644 --- a/monitor.c +++ b/monitor.c @@ -213,7 +213,6 @@ struct Monitor { BlockCompletionFunc *password_completion_cb; void *password_opaque; mon_cmd_t *cmd_table; -QLIST_HEAD(,mon_fd_t) fds; QTAILQ_ENTRY(Monitor) entry; /* @@ -225,6 +224,7 @@ struct Monitor { /* * Fields that are protected by the per-monitor lock. */ +QLIST_HEAD(, mon_fd_t) fds; QString *outbuf; guint out_watch; /* Read under either BQL or mon_lock, written with BQL+mon_lock. */ @@ -2189,7 +2189,7 @@ static void hmp_acl_remove(Monitor *mon, const QDict *qdict) void qmp_getfd(const char *fdname, Error **errp) { mon_fd_t *monfd; -int fd; +int fd, tmp_fd; fd = qemu_chr_fe_get_msgfd(_mon->chr); if (fd == -1) { @@ -2204,13 +2204,17 @@ void qmp_getfd(const char *fdname, Error **errp) return; } +qemu_mutex_lock(_mon->mon_lock); QLIST_FOREACH(monfd, _mon->fds, next) { if (strcmp(monfd->name, fdname) != 0) { continue; } -close(monfd->fd); +tmp_fd = monfd->fd; monfd->fd = fd; +qemu_mutex_unlock(_mon->mon_lock); +/* Make sure close() is out of critical section */ +close(tmp_fd); return; } @@ -2219,24 +2223,31 @@ void qmp_getfd(const char *fdname, Error **errp) monfd->fd = fd; QLIST_INSERT_HEAD(_mon->fds, monfd, next); +qemu_mutex_unlock(_mon->mon_lock); } void qmp_closefd(const char *fdname, Error **errp) { mon_fd_t *monfd; +int tmp_fd; +qemu_mutex_lock(_mon->mon_lock); QLIST_FOREACH(monfd, _mon->fds, next) { if (strcmp(monfd->name, fdname) != 0) { continue; } QLIST_REMOVE(monfd, next); -close(monfd->fd); +tmp_fd = monfd->fd; g_free(monfd->name); g_free(monfd); +qemu_mutex_unlock(_mon->mon_lock); +/* Make sure close() is out of critical section */ +close(tmp_fd); return; } +qemu_mutex_unlock(_mon->mon_lock); error_setg(errp, QERR_FD_NOT_FOUND, fdname); } @@ -2244,6 +2255,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) { mon_fd_t *monfd; +qemu_mutex_lock(>mon_lock); QLIST_FOREACH(monfd, >fds, next) { int fd; @@ -2257,10 +2269,12 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) QLIST_REMOVE(monfd, next); g_free(monfd->name); g_free(monfd); +qemu_mutex_unlock(>mon_lock); return fd; } +qemu_mutex_unlock(>mon_lock); error_setg(errp, "File descriptor named '%s' has not been found", fdname); return -1; } -- 2.17.0
[Qemu-devel] [PATCH v9 1/7] monitor: rename out_lock to mon_lock
The out_lock is protecting a few Monitor fields. In the future the monitor code will start to run in multiple threads. We are going to turn it into a bigger lock to protect not only the out buffer but also most of the rest. Since at it, rearrange the Monitor struct a bit. Reviewed-by: Stefan Hajnoczi Signed-off-by: Peter Xu --- monitor.c | 53 + 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/monitor.c b/monitor.c index 46814af533..14c681dc8a 100644 --- a/monitor.c +++ b/monitor.c @@ -207,15 +207,6 @@ struct Monitor { int suspend_cnt;/* Needs to be accessed atomically */ bool skip_flush; bool use_io_thr; - -/* We can't access guest memory when holding the lock */ -QemuMutex out_lock; -QString *outbuf; -guint out_watch; - -/* Read under either BQL or out_lock, written with BQL+out_lock. */ -int mux_out; - ReadLineState *rs; MonitorQMP qmp; gchar *mon_cpu_path; @@ -224,6 +215,20 @@ struct Monitor { mon_cmd_t *cmd_table; QLIST_HEAD(,mon_fd_t) fds; QTAILQ_ENTRY(Monitor) entry; + +/* + * The per-monitor lock. We can't access guest memory when holding + * the lock. + */ +QemuMutex mon_lock; + +/* + * Fields that are protected by the per-monitor lock. + */ +QString *outbuf; +guint out_watch; +/* Read under either BQL or mon_lock, written with BQL+mon_lock. */ +int mux_out; }; /* Let's add monitor global variables to this struct. */ @@ -366,14 +371,14 @@ static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond, { Monitor *mon = opaque; -qemu_mutex_lock(>out_lock); +qemu_mutex_lock(>mon_lock); mon->out_watch = 0; monitor_flush_locked(mon); -qemu_mutex_unlock(>out_lock); +qemu_mutex_unlock(>mon_lock); return FALSE; } -/* Called with mon->out_lock held. */ +/* Called with mon->mon_lock held. */ static void monitor_flush_locked(Monitor *mon) { int rc; @@ -411,9 +416,9 @@ static void monitor_flush_locked(Monitor *mon) void monitor_flush(Monitor *mon) { -qemu_mutex_lock(>out_lock); +qemu_mutex_lock(>mon_lock); monitor_flush_locked(mon); -qemu_mutex_unlock(>out_lock); +qemu_mutex_unlock(>mon_lock); } /* flush at every end of line */ @@ -421,7 +426,7 @@ static void monitor_puts(Monitor *mon, const char *str) { char c; -qemu_mutex_lock(>out_lock); +qemu_mutex_lock(>mon_lock); for(;;) { c = *str++; if (c == '\0') @@ -434,7 +439,7 @@ static void monitor_puts(Monitor *mon, const char *str) monitor_flush_locked(mon); } } -qemu_mutex_unlock(>out_lock); +qemu_mutex_unlock(>mon_lock); } void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) @@ -725,7 +730,7 @@ static void monitor_data_init(Monitor *mon, bool skip_flush, bool use_io_thr) { memset(mon, 0, sizeof(Monitor)); -qemu_mutex_init(>out_lock); +qemu_mutex_init(>mon_lock); qemu_mutex_init(>qmp.qmp_queue_lock); mon->outbuf = qstring_new(); /* Use *mon_cmds by default. */ @@ -745,7 +750,7 @@ static void monitor_data_destroy(Monitor *mon) } readline_free(mon->rs); qobject_unref(mon->outbuf); -qemu_mutex_destroy(>out_lock); +qemu_mutex_destroy(>mon_lock); qemu_mutex_destroy(>qmp.qmp_queue_lock); monitor_qmp_cleanup_req_queue_locked(mon); monitor_qmp_cleanup_resp_queue_locked(mon); @@ -777,13 +782,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, handle_hmp_command(, command_line); cur_mon = old_mon; -qemu_mutex_lock(_lock); +qemu_mutex_lock(_lock); if (qstring_get_length(hmp.outbuf) > 0) { output = g_strdup(qstring_get_str(hmp.outbuf)); } else { output = g_strdup(""); } -qemu_mutex_unlock(_lock); +qemu_mutex_unlock(_lock); out: monitor_data_destroy(); @@ -4377,9 +4382,9 @@ static void monitor_event(void *opaque, int event) switch (event) { case CHR_EVENT_MUX_IN: -qemu_mutex_lock(>out_lock); +qemu_mutex_lock(>mon_lock); mon->mux_out = 0; -qemu_mutex_unlock(>out_lock); +qemu_mutex_unlock(>mon_lock); if (mon->reset_seen) { readline_restart(mon->rs); monitor_resume(mon); @@ -4399,9 +4404,9 @@ static void monitor_event(void *opaque, int event) } else { atomic_inc(>suspend_cnt); } -qemu_mutex_lock(>out_lock); +qemu_mutex_lock(>mon_lock); mon->mux_out = 1; -qemu_mutex_unlock(>out_lock); +qemu_mutex_unlock(>mon_lock); break; case CHR_EVENT_OPENED: -- 2.17.0
Re: [Qemu-devel] [PATCH v7 4/4] monitor: add lock to protect mon_fdsets
On Mon, May 28, 2018 at 05:19:08PM +0200, Markus Armbruster wrote: [...] > >> > >> > + * Meanwhile it can also be used even at the end of main. Let's keep > >> > + * it initialized for the whole lifecycle of QEMU. > >> > + */ > >> > >> Awkward question, since our main() is such a tangled mess, but here goes > >> anyway... The existing place to initialize monitor.c's globals is > >> monitor_init_globals(). But that one runs too late, I guess: > >> parse_add_fd() runs earlier, and calls monitor_fdset_add_fd(). Unclean > >> even without this lock; no module should be used before its > >> initialization function runs. Sure we can't run monitor_init_globals() > >> sufficiently early? > > > > Please see the comment for monitor_init_globals(): > > > > /* > > * Note: qtest_enabled() (which is used in monitor_qapi_event_init()) > > * depends on configure_accelerator() above. > > */ > > monitor_init_globals(); > > > > So I guess it won't work to directly move it earlier. The init > > dependency of QEMU is really complicated. I'll be fine now that we > > mark totally independent init functions (like this one, which is a > > mutex init only) as constructor, then we can save some time on > > ordering issue. > > Let me rephrase. There's a preexisting issue: main() calls monitor.c > functions before calling its initialization function > monitor_init_globals(). This needs to be cleaned up. Would you be > willing to do it? > > Possible solutions: > > * Perhaps we can move monitor_init_globals() up and/or the code calling > into monitor.c early down sufficiently. > > * Calculate event_clock_type on each use instead of ahead of time. It's > qtest_enabled ? QEMU_CLOCK_VIRTUAL : QEMU_CLOCK_REALTIME, and neither > of its users needs to be fast. Then move monitor_init_globals before > the code calling into monitor.c. Indeed. Obviously you thought a step further. :) > > I'm not opposed to use of constructors for self-contained initialization > (no calls to other modules). But I don't like initialization spread > over multiple functions. Since this work will actually decide where I should init this new fdset lock, so I'll try to do that altogether within the series. Thanks for your suggestions! It makes sense. -- Peter Xu
[Qemu-devel] [PATCH] pc-bios/s390-ccw: define loadparm length
Loadparm is defined by the s390 architecture to be 8 bytes in length. Let's define this size in the s390-ccw bios. Suggested-by: Laszlo Ersek Signed-off-by: Collin Walling --- pc-bios/s390-ccw/iplb.h | 4 +++- pc-bios/s390-ccw/main.c | 8 pc-bios/s390-ccw/sclp.c | 2 +- pc-bios/s390-ccw/sclp.h | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h index ded20c8..772d5c5 100644 --- a/pc-bios/s390-ccw/iplb.h +++ b/pc-bios/s390-ccw/iplb.h @@ -12,6 +12,8 @@ #ifndef IPLB_H #define IPLB_H +#define LOADPARM_LEN8 + struct IplBlockCcw { uint8_t reserved0[85]; uint8_t ssid; @@ -61,7 +63,7 @@ struct IplParameterBlock { uint8_t pbt; uint8_t flags; uint16_t reserved01; -uint8_t loadparm[8]; +uint8_t loadparm[LOADPARM_LEN]; union { IplBlockCcw ccw; IplBlockFcp fcp; diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index 26f9adf..544851d 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -15,7 +15,7 @@ char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); static SubChannelId blk_schid = { .one = 1 }; IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); -static char loadparm_str[9] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }; +static char loadparm_str[LOADPARM_LEN + 1] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }; QemuIplParameters qipl; #define LOADPARM_PROMPT "PROMPT " @@ -80,13 +80,13 @@ static bool find_dev(Schib *schib, int dev_no) static void menu_setup(void) { -if (memcmp(loadparm_str, LOADPARM_PROMPT, 8) == 0) { +if (memcmp(loadparm_str, LOADPARM_PROMPT, LOADPARM_LEN) == 0) { menu_set_parms(QIPL_FLAG_BM_OPTS_CMD, 0); return; } /* If loadparm was set to any other value, then do not enable menu */ -if (memcmp(loadparm_str, LOADPARM_EMPTY, 8) != 0) { +if (memcmp(loadparm_str, LOADPARM_EMPTY, LOADPARM_LEN) != 0) { return; } @@ -117,7 +117,7 @@ static void virtio_setup(void) enable_mss_facility(); sclp_get_loadparm_ascii(loadparm_str); -memcpy(ldp + 10, loadparm_str, 8); +memcpy(ldp + 10, loadparm_str, LOADPARM_LEN); sclp_print(ldp); memcpy(, early_qipl, sizeof(QemuIplParameters)); diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c index 3836cb4..c0223fa 100644 --- a/pc-bios/s390-ccw/sclp.c +++ b/pc-bios/s390-ccw/sclp.c @@ -114,7 +114,7 @@ void sclp_get_loadparm_ascii(char *loadparm) memset((char *)_sccb, 0, sizeof(ReadInfo)); sccb->h.length = sizeof(ReadInfo); if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) { -ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8); +ebcdic_to_ascii((char *) sccb->loadparm, loadparm, LOADPARM_LEN); } } diff --git a/pc-bios/s390-ccw/sclp.h b/pc-bios/s390-ccw/sclp.h index 0dd987f..8450161 100644 --- a/pc-bios/s390-ccw/sclp.h +++ b/pc-bios/s390-ccw/sclp.h @@ -56,7 +56,7 @@ typedef struct ReadInfo { uint16_t rnmax; uint8_t rnsize; uint8_t reserved[13]; -uint8_t loadparm[8]; +uint8_t loadparm[LOADPARM_LEN]; } __attribute__((packed)) ReadInfo; typedef struct SCCB { -- 2.7.4
Re: [Qemu-devel] [PATCH v2] aspeed_scu: Implement RNG register
On 29 May 2018 at 01:37, Philippe Mathieu-Daudé wrote: > Hi Joel, > > On 05/28/2018 12:22 PM, Joel Stanley wrote: >> The ASPEED SoCs contain a single register that returns random data when >> read. This models that register so that guests can use it. >> >> The random number data register has a corresponding control register, >> data returns a different number regardless of the state of the enabled >> bit, so the model follows this behaviour. > > I have been bugging Cédric to check specs for the RNG_CTRL register and > he sent me the v1 of this patch than I missed: > > >>> may be we could test bit 1 of RNG_CTRL to check if it is enabled > or not. > >> > >> The RNG is enabled by default, and I didn't find any software that > >> disables it, but it can't hurt to have that check. > > > > I did some testing on hardware, and the rng still outputs a different > > number each time I ask for one regardless of the state of the enabled > > bit. > > > I confirm that the HW doesn't really care about the enabled bit. > Let's ignore it then ? > > Now your comment makes more sens, however I think it would be more > useful to add it in aspeed_scu_read() to make it obvious. > >> >> Signed-off-by: Joel Stanley >> --- >> v2: >> - Remove call to qcrypto_random_init as this is done in main() >> --- >> hw/misc/aspeed_scu.c | 16 >> 1 file changed, 16 insertions(+) >> >> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c >> index 5e6d5744eeca..29e58527793b 100644 >> --- a/hw/misc/aspeed_scu.c >> +++ b/hw/misc/aspeed_scu.c >> @@ -16,6 +16,7 @@ >> #include "qapi/visitor.h" >> #include "qemu/bitops.h" >> #include "qemu/log.h" >> +#include "crypto/random.h" >> #include "trace.h" >> >> #define TO_REG(offset) ((offset) >> 2) >> @@ -154,6 +155,18 @@ static const uint32_t >> ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { >> [BMC_DEV_ID] = 0x2402U >> }; >> >> +static uint32_t aspeed_scu_get_random(void) >> +{ >> +Error *err = NULL; >> +uint32_t num; >> + >> +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { >> +error_report_err(err); >> +} >> + >> +return num; >> +} > > This function duplicates bcm2835_rng::get_random_bytes() except it > doesn't exit(), is that on purpose? Yes. It's not fatal to the emulation if we can't provide random data. The aspeed model is used for smoketesting firmware builds and aiding in firmware development, so strong cryptographic guarantees are not essential. > What about refactoring, inlining bcm2835_rng::get_random_bytes() in a > new include/hw/misc/rng.h? > > (This can be later patch) > >> + >> static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) >> { >> AspeedSCUState *s = ASPEED_SCU(opaque); >> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr >> offset, unsigned size) >> } >> >> switch (reg) { > >case RNG_CTRL: >/* The RNG_DATA returns a different number regardless of > * the state of the enabled bit in RNG_CTRL, > * so the model follows this behaviour. > */ >break; > > With a comment: > Reviewed-by: Philippe Mathieu-Daudé Thanks. I don't think it makes sense to do this for the read case, and it was strange to find it in the write callback, so I instead added a comment next to RNG_DATA. Cheers, Joel
[Qemu-devel] [PATCH v3] aspeed_scu: Implement RNG register
The ASPEED SoCs contain a single register that returns random data when read. This models that register so that guests can use it. The random number data register has a corresponding control register, however it returns data regardless of the state of the enabled bit, so the model follows this behaviour. Reviewed-by: Cédric Le Goater Signed-off-by: Joel Stanley --- v2: - Remove call to qcrypto_random_init as this is done in main() v3: - Add Cédric's reviewed-by - Add a comment about why we don't check for the rng enable bit --- hw/misc/aspeed_scu.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 5e6d5744eeca..96db052389cc 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -16,6 +16,7 @@ #include "qapi/visitor.h" #include "qemu/bitops.h" #include "qemu/log.h" +#include "crypto/random.h" #include "trace.h" #define TO_REG(offset) ((offset) >> 2) @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { [BMC_DEV_ID] = 0x2402U }; +static uint32_t aspeed_scu_get_random(void) +{ +Error *err = NULL; +uint32_t num; + +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { +error_report_err(err); +} + +return num; +} + static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); @@ -167,6 +180,12 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) } switch (reg) { +case RNG_DATA: +/* On hardware, RNG_DATA works regardless of + * the state of the enable bit in RNG_CTRL + */ +s->regs[RNG_DATA] = aspeed_scu_get_random(); +break; case WAKEUP_EN: qemu_log_mask(LOG_GUEST_ERROR, "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", -- 2.17.0
Re: [Qemu-devel] [PATCH v6 04/10] qcow2: Implement copy offloading
On Mon, 05/28 11:36, Fam Zheng wrote: > The two callbacks are implemented quite similarly to the read/write > functions: bdrv_co_copy_range_from maps for read and calls into bs->file > or bs->backing depending on the allocation status; bdrv_co_copy_range_to > maps for write and calls into bs->file. > > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Fam Zheng > --- > block/qcow2.c | 226 > ++ > 1 file changed, 196 insertions(+), 30 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 6d532470a8..e32a3c1518 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1762,6 +1762,39 @@ static int coroutine_fn > qcow2_co_block_status(BlockDriverState *bs, > return status; > } > > +static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs, > +QCowL2Meta **pl2meta, > +bool link_l2) > +{ > +int ret = 0; > +QCowL2Meta *l2meta = *pl2meta; > + > +while (l2meta != NULL) { > +QCowL2Meta *next; > + > +if (!ret && link_l2) { > +ret = qcow2_alloc_cluster_link_l2(bs, l2meta); > +if (ret) { > +goto out; > +} > +} > + > +/* Take the request off the list of running requests */ > +if (l2meta->nb_clusters != 0) { > +QLIST_REMOVE(l2meta, next_in_flight); > +} > + > +qemu_co_queue_restart_all(>dependent_requests); > + > +next = l2meta->next; > +g_free(l2meta); > +l2meta = next; > +} > +out: > +*pl2meta = l2meta; > +return ret; > +} > + > static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t > offset, > uint64_t bytes, QEMUIOVector *qiov, > int flags) > @@ -2048,24 +2081,9 @@ static coroutine_fn int > qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset, > } > } > > -while (l2meta != NULL) { > -QCowL2Meta *next; > - > -ret = qcow2_alloc_cluster_link_l2(bs, l2meta); > -if (ret < 0) { > -goto fail; > -} > - > -/* Take the request off the list of running requests */ > -if (l2meta->nb_clusters != 0) { > -QLIST_REMOVE(l2meta, next_in_flight); > -} > - > -qemu_co_queue_restart_all(>dependent_requests); > - > -next = l2meta->next; > -g_free(l2meta); > -l2meta = next; > +ret = qcow2_handle_l2meta(bs, , true); > +if (ret) { > +goto fail; > } > > bytes -= cur_bytes; > @@ -2076,18 +2094,7 @@ static coroutine_fn int > qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset, > ret = 0; > > fail: > -while (l2meta != NULL) { > -QCowL2Meta *next; > - > -if (l2meta->nb_clusters != 0) { > -QLIST_REMOVE(l2meta, next_in_flight); > -} > -qemu_co_queue_restart_all(>dependent_requests); > - > -next = l2meta->next; > -g_free(l2meta); > -l2meta = next; > -} > +qcow2_handle_l2meta(bs, , false); > > qemu_co_mutex_unlock(>lock); > > @@ -3274,6 +3281,163 @@ static coroutine_fn int > qcow2_co_pdiscard(BlockDriverState *bs, > return ret; > } > > +static int coroutine_fn > +qcow2_co_copy_range_from(BlockDriverState *bs, > + BdrvChild *src, uint64_t src_offset, > + BdrvChild *dst, uint64_t dst_offset, > + uint64_t bytes, BdrvRequestFlags flags) > +{ > +BDRVQcow2State *s = bs->opaque; > +int ret; > +unsigned int cur_bytes; /* number of bytes in current iteration */ > +BdrvChild *child = NULL; > + > +assert(!bs->encrypted); > +qemu_co_mutex_lock(>lock); > + > +while (bytes != 0) { NACK. flags and dst_offset are calculated wrong in the loop body: bits in the flags are not reset when starting a new iteration; dst_offset is not incremented. These bugs are caught during testing the follow up drive-backup patches. Will send v7 to fix it. Fam > +uint64_t copy_offset = 0; > +/* prepare next request */ > +cur_bytes = MIN(bytes, INT_MAX); > + > +ret = qcow2_get_cluster_offset(bs, src_offset, _bytes, > _offset); > +if (ret < 0) { > +goto out; > +} > + > +switch (ret) { > +case QCOW2_CLUSTER_UNALLOCATED: > +if (bs->backing && bs->backing->bs) { > +int64_t backing_length = bdrv_getlength(bs->backing->bs); > +if (src_offset >= backing_length) { > +flags |= BDRV_REQ_ZERO_WRITE; > +} else { > +child = bs->backing; > +cur_bytes = MIN(cur_bytes, backing_length - src_offset); > +
[Qemu-devel] [PATCH v2] ARM: ACPI: Fix use-after-free due to memory realloc
acpi_data_push uses g_array_set_size to resize the memory size. If there is no enough contiguous memory, the address will be changed. So previous pointer could not be used any more. It must update the pointer and use the new one. Reviewed-by: Eric Auger Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Shannon Zhao --- V2: add comments for iort_node_offset and reviewed tags --- hw/arm/virt-acpi-build.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 92ceee9..6209138 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -400,7 +400,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) AcpiIortItsGroup *its; AcpiIortTable *iort; AcpiIortSmmu3 *smmu; -size_t node_size, iort_length, smmu_offset = 0; +size_t node_size, iort_node_offset, iort_length, smmu_offset = 0; AcpiIortRC *rc; iort = acpi_data_push(table_data, sizeof(*iort)); @@ -415,6 +415,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) iort->node_count = cpu_to_le32(nb_nodes); iort->node_offset = cpu_to_le32(sizeof(*iort)); +/* + * Use a copy in case table_data->data moves duringa acpi_data_push + * operations. + */ +iort_node_offset = sizeof(*iort); + /* ITS group node */ node_size = sizeof(*its) + sizeof(uint32_t); iort_length += node_size; @@ -429,7 +435,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) int irq = vms->irqmap[VIRT_SMMU]; /* SMMUv3 node */ -smmu_offset = iort->node_offset + node_size; +smmu_offset = iort_node_offset + node_size; node_size = sizeof(*smmu) + sizeof(*idmap); iort_length += node_size; smmu = acpi_data_push(table_data, node_size); @@ -450,7 +456,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) idmap->id_count = cpu_to_le32(0x); idmap->output_base = 0; /* output IORT node is the ITS group node (the first node) */ -idmap->output_reference = cpu_to_le32(iort->node_offset); +idmap->output_reference = cpu_to_le32(iort_node_offset); } /* Root Complex Node */ @@ -479,9 +485,14 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) idmap->output_reference = cpu_to_le32(smmu_offset); } else { /* output IORT node is the ITS group node (the first node) */ -idmap->output_reference = cpu_to_le32(iort->node_offset); +idmap->output_reference = cpu_to_le32(iort_node_offset); } +/* + * Update the pointer address in case table_data->data moves during above + * acpi_data_push operations. + */ +iort = (AcpiIortTable *)(table_data->data + iort_start); iort->length = cpu_to_le32(iort_length); build_header(linker, table_data, (void *)(table_data->data + iort_start), -- 2.0.4
[Qemu-devel] [PATCH] memory: bug 1720969: Make operations using MemoryRegionIoeventfd struct pass by pointer.
This changes the functions memory_region_ioeventfd_equal, memory_region_ioeventfd_before, and their callers, to pass the MemoryRegionIoeventfd struct via pointer, instead of directly passing the struct. This saves on stack space and is considered safe practice. Signed-off-by: Tristan Burgess --- memory.c | 44 ++-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/memory.c b/memory.c index fc7f9b782b..a2b9bb88f3 100644 --- a/memory.c +++ b/memory.c @@ -173,38 +173,38 @@ struct MemoryRegionIoeventfd { EventNotifier *e; }; -static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a, - MemoryRegionIoeventfd b) +static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd *a, + MemoryRegionIoeventfd *b) { -if (int128_lt(a.addr.start, b.addr.start)) { +if (int128_lt(a->addr.start, b->addr.start)) { return true; -} else if (int128_gt(a.addr.start, b.addr.start)) { +} else if (int128_gt(a->addr.start, b->addr.start)) { return false; -} else if (int128_lt(a.addr.size, b.addr.size)) { +} else if (int128_lt(a->addr.size, b->addr.size)) { return true; -} else if (int128_gt(a.addr.size, b.addr.size)) { +} else if (int128_gt(a->addr.size, b->addr.size)) { return false; -} else if (a.match_data < b.match_data) { +} else if (a->match_data < b->match_data) { return true; -} else if (a.match_data > b.match_data) { +} else if (a->match_data > b->match_data) { return false; -} else if (a.match_data) { -if (a.data < b.data) { +} else if (a->match_data) { +if (a->data < b->data) { return true; -} else if (a.data > b.data) { +} else if (a->data > b->data) { return false; } } -if (a.e < b.e) { +if (a->e < b->e) { return true; -} else if (a.e > b.e) { +} else if (a->e > b->e) { return false; } return false; } -static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a, - MemoryRegionIoeventfd b) +static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd *a, + MemoryRegionIoeventfd *b) { return !memory_region_ioeventfd_before(a, b) && !memory_region_ioeventfd_before(b, a); @@ -791,8 +791,8 @@ static void address_space_add_del_ioeventfds(AddressSpace *as, while (iold < fds_old_nb || inew < fds_new_nb) { if (iold < fds_old_nb && (inew == fds_new_nb -|| memory_region_ioeventfd_before(fds_old[iold], - fds_new[inew]))) { +|| memory_region_ioeventfd_before(_old[iold], + _new[inew]))) { fd = _old[iold]; section = (MemoryRegionSection) { .fv = address_space_to_flatview(as), @@ -804,8 +804,8 @@ static void address_space_add_del_ioeventfds(AddressSpace *as, ++iold; } else if (inew < fds_new_nb && (iold == fds_old_nb - || memory_region_ioeventfd_before(fds_new[inew], - fds_old[iold]))) { + || memory_region_ioeventfd_before(_new[inew], + _old[iold]))) { fd = _new[inew]; section = (MemoryRegionSection) { .fv = address_space_to_flatview(as), @@ -1443,7 +1443,7 @@ static bool memory_region_dispatch_write_eventfds(MemoryRegion *mr, ioeventfd.match_data = mr->ioeventfds[i].match_data; ioeventfd.e = mr->ioeventfds[i].e; -if (memory_region_ioeventfd_equal(ioeventfd, mr->ioeventfds[i])) { +if (memory_region_ioeventfd_equal(, >ioeventfds[i])) { event_notifier_set(ioeventfd.e); return true; } @@ -2213,7 +2213,7 @@ void memory_region_add_eventfd(MemoryRegion *mr, } memory_region_transaction_begin(); for (i = 0; i < mr->ioeventfd_nb; ++i) { -if (memory_region_ioeventfd_before(mrfd, mr->ioeventfds[i])) { +if (memory_region_ioeventfd_before(, >ioeventfds[i])) { break; } } @@ -2248,7 +2248,7 @@ void memory_region_del_eventfd(MemoryRegion *mr, } memory_region_transaction_begin(); for (i = 0; i < mr->ioeventfd_nb; ++i) { -if (memory_region_ioeventfd_equal(mrfd, mr->ioeventfds[i])) { +if (memory_region_ioeventfd_equal(, >ioeventfds[i])) { break; } } -- 2.17.0
[Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
Signed-off-by: linzhecheng diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 159e69c3b1..17519ec589 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len) s->write_msgfds, s->write_msgfds_num); -/* free the written msgfds, no matter what */ -if (s->write_msgfds_num) { +/* free the written msgfds in any cases other than errno==EAGAIN */ +if (EAGAIN != errno && s->write_msgfds_num) { g_free(s->write_msgfds); s->write_msgfds = 0; s->write_msgfds_num = 0; -- 2.12.2.windows.2
[Qemu-devel] [Bug 1773753] Re: virsh start after virsh managed save hangs and vm goes to paused state with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc
** Summary changed: - virsh managed save fails with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc + virsh start after virsh managed save hangs and vm goes to paused state with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc ** Summary changed: - virsh start after virsh managed save hangs and vm goes to paused state with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc + virsh start, after virsh managed save hangs and vm goes to paused state with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1773753 Title: virsh start, after virsh managed save hangs and vm goes to paused state with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc Status in QEMU: New Bug description: Host Env: IBM Power8 with Fedora28 base with compiled upstream kernel, qemu, libvirt. Host Kernel: 4.17.0-rc5-00069-g3acf4e395260 qemu-kvm(5a5c383b1373aeb6c87a0d6060f6c3dc7c53082b): v2.12.0-813-g5a5c383b13-dirty libvirt(4804a4db33a37f828d033733bc47f6eff5d262c3): Guest Kernel: 4.17.0-rc7 Steps to recreate: Define a guest attached with above setup and start. # virsh start avocado-vt-vm1 guest console;... # uname -r 4.17.0-rc7 [root@atest-guest ~]# lscpu Architecture:ppc64le Byte Order: Little Endian CPU(s): 3 On-line CPU(s) list: 0-2 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s): 3 NUMA node(s):1 Model: 2.1 (pvr 004b 0201) Model name: POWER8 (architected), altivec supported Hypervisor vendor: KVM Virtualization type: para L1d cache: 64K L1i cache: 32K NUMA node0 CPU(s): 0-2 # virsh managedsave avocado-vt-vm1 Domain avocado-vt-vm1 state saved by libvirt # virsh list IdName State # virsh start avocado-vt-vm1 Hangs forever and vm state goes to paused. # virsh list IdName State 87avocado-vt-vm1 paused P:S:- with same above setup, just changing the qemu-kvm comes bydefault with F28 works fine. /usr/bin/qemu-kvm --version QEMU emulator version 2.11.1(qemu-2.11.1-2.fc28) Summary: with above other setup. machine type pseries-2.12 and qemu-2.11.1-2.fc28 -Works fine. machine type pseries-2.12/pseries-2.13 and qemu 5a5c383b1373aeb6c87a0d6060f6c3dc7c53082b - Does not work. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1773753/+subscriptions
[Qemu-devel] [PATCH v2] vhost-blk: turn on pre-defined RO feature bit
Read only feature shouldn't be negotiable, because if the backend device reported Read only feature supported, QEMU host driver shouldn't change backend's RO attribute. While here, also enable the vhost-user-blk test utility to test RO feature. Signed-off-by: Changpeng Liu --- contrib/vhost-user-blk/vhost-user-blk.c | 48 - hw/block/vhost-user-blk.c | 5 +--- include/hw/virtio/vhost-user-blk.h | 1 - 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c index 67dac81..f57df45 100644 --- a/contrib/vhost-user-blk/vhost-user-blk.c +++ b/contrib/vhost-user-blk/vhost-user-blk.c @@ -31,6 +31,7 @@ typedef struct VubDev { VugDev parent; int blk_fd; struct virtio_blk_config blkcfg; +bool enable_ro; char *blk_name; GMainLoop *loop; } VubDev; @@ -301,14 +302,27 @@ static void vub_queue_set_started(VuDev *vu_dev, int idx, bool started) static uint64_t vub_get_features(VuDev *dev) { -return 1ull << VIRTIO_BLK_F_SIZE_MAX | - 1ull << VIRTIO_BLK_F_SEG_MAX | - 1ull << VIRTIO_BLK_F_TOPOLOGY | - 1ull << VIRTIO_BLK_F_BLK_SIZE | - 1ull << VIRTIO_BLK_F_FLUSH | - 1ull << VIRTIO_BLK_F_CONFIG_WCE | - 1ull << VIRTIO_F_VERSION_1 | - 1ull << VHOST_USER_F_PROTOCOL_FEATURES; +uint64_t features; +VugDev *gdev; +VubDev *vdev_blk; + +gdev = container_of(dev, VugDev, parent); +vdev_blk = container_of(gdev, VubDev, parent); + +features = 1ull << VIRTIO_BLK_F_SIZE_MAX | + 1ull << VIRTIO_BLK_F_SEG_MAX | + 1ull << VIRTIO_BLK_F_TOPOLOGY | + 1ull << VIRTIO_BLK_F_BLK_SIZE | + 1ull << VIRTIO_BLK_F_FLUSH | + 1ull << VIRTIO_BLK_F_CONFIG_WCE | + 1ull << VIRTIO_F_VERSION_1 | + 1ull << VHOST_USER_F_PROTOCOL_FEATURES; + +if (vdev_blk->enable_ro) { +features |= 1ull << VIRTIO_BLK_F_RO; +} + +return features; } static int @@ -469,6 +483,7 @@ vub_new(char *blk_file) vub_free(vdev_blk); return NULL; } +vdev_blk->enable_ro = false; vdev_blk->blkcfg.wce = 0; vdev_blk->blk_name = blk_file; @@ -483,10 +498,11 @@ int main(int argc, char **argv) int opt; char *unix_socket = NULL; char *blk_file = NULL; +bool enable_ro = false; int lsock = -1, csock = -1; VubDev *vdev_blk = NULL; -while ((opt = getopt(argc, argv, "b:s:h")) != -1) { +while ((opt = getopt(argc, argv, "b:rs:h")) != -1) { switch (opt) { case 'b': blk_file = g_strdup(optarg); @@ -494,17 +510,20 @@ int main(int argc, char **argv) case 's': unix_socket = g_strdup(optarg); break; +case 'r': +enable_ro = true; +break; case 'h': default: -printf("Usage: %s [-b block device or file, -s UNIX domain socket]" - " | [ -h ]\n", argv[0]); +printf("Usage: %s [ -b block device or file, -s UNIX domain socket" + " | -r Enable read-only ] | [ -h ]\n", argv[0]); return 0; } } if (!unix_socket || !blk_file) { -printf("Usage: %s [-b block device or file, -s UNIX domain socket] |" - " [ -h ]\n", argv[0]); +printf("Usage: %s [ -b block device or file, -s UNIX domain socket" + " | -r Enable read-only ] | [ -h ]\n", argv[0]); return -1; } @@ -523,6 +542,9 @@ int main(int argc, char **argv) if (!vdev_blk) { goto err; } +if (enable_ro) { +vdev_blk->enable_ro = true; +} vug_init(_blk->parent, csock, vub_panic_cb, _iface); diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 975eae6..e32e50c 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -203,13 +203,11 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev, virtio_add_feature(, VIRTIO_BLK_F_TOPOLOGY); virtio_add_feature(, VIRTIO_BLK_F_BLK_SIZE); virtio_add_feature(, VIRTIO_BLK_F_FLUSH); +virtio_add_feature(, VIRTIO_BLK_F_RO); if (s->config_wce) { virtio_add_feature(, VIRTIO_BLK_F_CONFIG_WCE); } -if (s->config_ro) { -virtio_add_feature(, VIRTIO_BLK_F_RO); -} if (s->num_queues > 1) { virtio_add_feature(, VIRTIO_BLK_F_MQ); } @@ -319,7 +317,6 @@ static Property vhost_user_blk_properties[] = { DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues, 1), DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128), DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true), -DEFINE_PROP_BIT("config-ro", VHostUserBlk, config_ro, 0, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/vhost-user-blk.h
Re: [Qemu-devel] [PATCH v4 15/21] hw/block/nvme: Include "qemu/cutils.h" directly in the source file
On 05/28/2018 08:27 PM, Philippe Mathieu-Daudé wrote: I forgot to add this commit description: "block/nvme.h" does not require any declaration of "qemu/cutils.h". Simplify dependencies by directly include it in the source file where the declarations are used. > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Thomas Huth > --- > hw/block/nvme.h | 1 - > hw/block/nvme.c | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/block/nvme.h b/hw/block/nvme.h > index 8f3981121d..cabcf20c32 100644 > --- a/hw/block/nvme.h > +++ b/hw/block/nvme.h > @@ -1,6 +1,5 @@ > #ifndef HW_NVME_H > #define HW_NVME_H > -#include "qemu/cutils.h" > #include "block/nvme.h" > > typedef struct NvmeAsyncEvent { > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 85d2406400..811084b6a7 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -35,6 +35,7 @@ > #include "sysemu/block-backend.h" > > #include "qemu/log.h" > +#include "qemu/cutils.h" > #include "trace.h" > #include "nvme.h" > >
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On 05/28/2018 09:06 PM, Michael S. Tsirkin wrote: > On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: >> On Mon, 28 May 2018 20:26:59 -0300 >> Philippe Mathieu-Daudé wrote: >> >> -ENOCOMMITLOG Oops sorry Alex, I meant to add some, but missed this while rebasing. >> Why? Tangible benefit. Looks like noise. Thanks, >> > I agree it should have a commit log, but .c files > should be self-sufficient not rely on .h files > pulling in headers for symbols the .h does not use > itself. I meant: No declaration of "hw/vfio/vfio-common.h" directly requires to include the "exec/address-spaces.h" header. To simplify dependencies and ease following cleanup of "exec/address-spaces.h", directly include it in the source file where the declaration are used. > This is better because it makes refactoring easier. > >>> Signed-off-by: Philippe Mathieu-Daudé >>> --- >>> include/hw/vfio/vfio-common.h | 1 - >>> hw/vfio/ccw.c | 1 + >>> hw/vfio/platform.c| 1 + >>> 3 files changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >>> index d9360148e6..8264a65fa5 100644 >>> --- a/include/hw/vfio/vfio-common.h >>> +++ b/include/hw/vfio/vfio-common.h >>> @@ -22,7 +22,6 @@ >>> #define HW_VFIO_VFIO_COMMON_H >>> >>> #include "qemu-common.h" >>> -#include "exec/address-spaces.h" >>> #include "exec/memory.h" >>> #include "qemu/queue.h" >>> #include "qemu/notify.h" >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c >>> index e67392c5f9..76e4e8c652 100644 >>> --- a/hw/vfio/ccw.c >>> +++ b/hw/vfio/ccw.c >>> @@ -22,6 +22,7 @@ >>> #include "hw/vfio/vfio-common.h" >>> #include "hw/s390x/s390-ccw.h" >>> #include "hw/s390x/ccw-device.h" >>> +#include "exec/address-spaces.h" >>> #include "qemu/error-report.h" >>> >>> #define TYPE_VFIO_CCW "vfio-ccw" >>> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c >>> index 5c921c27ba..57c4a0ee2b 100644 >>> --- a/hw/vfio/platform.c >>> +++ b/hw/vfio/platform.c >>> @@ -24,6 +24,7 @@ >>> #include "qemu/range.h" >>> #include "sysemu/sysemu.h" >>> #include "exec/memory.h" >>> +#include "exec/address-spaces.h" >>> #include "qemu/queue.h" >>> #include "hw/sysbus.h" >>> #include "trace.h"
Re: [Qemu-devel] [PATCH v4 00/21] Includes cleanup
On Mon, May 28, 2018 at 08:26:58PM -0300, Philippe Mathieu-Daudé wrote: > Hi, > > I split the previous series "Use the BYTE-based definitions when useful", > this is the first generic part, only headers cleanup, which is big enough. > > Many patches, but "12 insertions(+), 145 deletions(-)" \o/ Series Acked-by: Michael S. Tsirkin > v3 was: > http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02049.html > > Philippe Mathieu-Daudé (21): > vfio: Include "exec/address-spaces.h" directly in the source file > accel: Do not include "exec/address-spaces.h" if it is not necessary > target: Do not include "exec/address-spaces.h" if it is not necessary > memory: Do not include "exec/ioport.h" if it is not necessary > target/i386: Do not include "exec/ioport.h" if it is not necessary > target/xtensa: Include "qemu/timer.h" to use NANOSECONDS_PER_SECOND > target/ppc: Include "exec/exec-all.h" which provides tlb_flush() > target/hppa: Include "qemu/log.h" to use qemu_log() > target: Do not include "exec/exec-all.h" if it is not necessary > hw: Do not include "exec/ioport.h" if it is not necessary > hw: Do not include "exec/address-spaces.h" if it is not necessary > hw: Do not include "sysemu/block-backend.h" if it is not necessary > hw: Do not include "sysemu/blockdev.h" if it is not necessary > hw: Do not include "sysemu/blockdev.h" if it is not necessary > hw/block/nvme: Include "qemu/cutils.h" directly in the source file > hw/misc/mips_itu: Cleanup includes > hw/misc/sga: Use the correct ISA include > hw/hppa: Remove unused include > hw/i386/pc: Remove unused include > hw/ide: Remove unused include > hw: Clean "hw/devices.h" includes > > bsd-user/qemu.h | 1 - > hw/block/nvme.h | 1 - > hw/hppa/hppa_sys.h | 1 - > include/hw/arm/allwinner-a10.h | 1 - > include/hw/arm/bcm2835_peripherals.h | 1 - > include/hw/devices.h | 7 ++- > include/hw/display/bcm2835_fb.h | 1 - > include/hw/dma/bcm2835_dma.h | 1 - > include/hw/misc/bcm2835_mbox.h | 1 - > include/hw/misc/bcm2835_property.h | 1 - > include/hw/misc/mips_itu.h | 2 ++ > include/hw/sh4/sh_intc.h | 1 - > include/hw/vfio/vfio-common.h| 1 - > include/hw/virtio/virtio-access.h| 1 - > target/arm/arm_ldst.h| 1 - > target/ppc/helper_regs.h | 1 + > accel/tcg/cpu-exec.c | 1 - > hw/acpi/pcihp.c | 1 - > hw/acpi/piix4.c | 1 - > hw/arm/aspeed.c | 1 - > hw/arm/bcm2836.c | 1 - > hw/arm/collie.c | 1 - > hw/arm/gumstix.c | 1 - > hw/arm/mainstone.c | 1 - > hw/arm/nseries.c | 1 - > hw/arm/omap1.c | 2 -- > hw/arm/omap2.c | 2 -- > hw/arm/omap_sx1.c| 1 - > hw/arm/pxa2xx.c | 1 - > hw/arm/spitz.c | 1 - > hw/arm/versatilepb.c | 1 - > hw/arm/vexpress.c| 1 - > hw/arm/virt.c| 1 - > hw/arm/xilinx_zynq.c | 1 - > hw/arm/xlnx-zcu102.c | 1 - > hw/arm/z2.c | 1 - > hw/block/dataplane/virtio-blk.c | 1 - > hw/block/m25p80.c| 1 - > hw/block/nvme.c | 1 + > hw/block/onenand.c | 2 -- > hw/block/pflash_cfi01.c | 1 - > hw/block/pflash_cfi02.c | 1 - > hw/block/virtio-blk.c| 1 - > hw/char/mcf_uart.c | 1 - > hw/char/serial.c | 1 - > hw/char/sh_serial.c | 1 - > hw/core/loader-fit.c | 1 - > hw/core/platform-bus.c | 1 - > hw/core/qdev-properties.c| 1 - > hw/cris/axis_dev88.c | 1 - > hw/display/sm501.c | 1 - > hw/display/tc6393xb.c| 1 - > hw/i386/kvmvapic.c | 1 - > hw/i386/pc.c | 1 - > hw/i386/xen/xen-mapcache.c | 1 - > hw/ide/core.c| 1 - > hw/ide/pci.c | 1 - > hw/ide/via.c | 1 - > hw/isa/isa-superio.c | 1 - > hw/lm32/lm32_boards.c| 1 - > hw/lm32/milkymist.c | 1 - > hw/m68k/mcf5206.c| 1 - > hw/m68k/mcf_intc.c | 1 - > hw/microblaze/petalogix_ml605_mmu.c | 1 - > hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 - > hw/mips/mips_malta.c
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Mon, May 28, 2018 at 05:48:05PM -0600, Alex Williamson wrote: > On Mon, 28 May 2018 20:26:59 -0300 > Philippe Mathieu-Daudé wrote: > > -ENOCOMMITLOG > > Why? Tangible benefit. Looks like noise. Thanks, > > Alex I agree it should have a commit log, but .c files should be self-sufficient not rely on .h files pulling in headers for symbols the .h does not use itself. This is better because it makes refactoring easier. > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > include/hw/vfio/vfio-common.h | 1 - > > hw/vfio/ccw.c | 1 + > > hw/vfio/platform.c| 1 + > > 3 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > > index d9360148e6..8264a65fa5 100644 > > --- a/include/hw/vfio/vfio-common.h > > +++ b/include/hw/vfio/vfio-common.h > > @@ -22,7 +22,6 @@ > > #define HW_VFIO_VFIO_COMMON_H > > > > #include "qemu-common.h" > > -#include "exec/address-spaces.h" > > #include "exec/memory.h" > > #include "qemu/queue.h" > > #include "qemu/notify.h" > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > > index e67392c5f9..76e4e8c652 100644 > > --- a/hw/vfio/ccw.c > > +++ b/hw/vfio/ccw.c > > @@ -22,6 +22,7 @@ > > #include "hw/vfio/vfio-common.h" > > #include "hw/s390x/s390-ccw.h" > > #include "hw/s390x/ccw-device.h" > > +#include "exec/address-spaces.h" > > #include "qemu/error-report.h" > > > > #define TYPE_VFIO_CCW "vfio-ccw" > > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > > index 5c921c27ba..57c4a0ee2b 100644 > > --- a/hw/vfio/platform.c > > +++ b/hw/vfio/platform.c > > @@ -24,6 +24,7 @@ > > #include "qemu/range.h" > > #include "sysemu/sysemu.h" > > #include "exec/memory.h" > > +#include "exec/address-spaces.h" > > #include "qemu/queue.h" > > #include "hw/sysbus.h" > > #include "trace.h"
[Qemu-devel] [PATCH v4 12/21] hw: Do not include "sysemu/block-backend.h" if it is not necessary
Remove those unneeded includes to speed up the compilation process a little bit. (Continue 7eceff5b5a1fa cleanup) Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/collie.c | 1 - hw/arm/gumstix.c | 1 - hw/arm/mainstone.c | 1 - hw/arm/nseries.c | 1 - hw/arm/omap1.c | 1 - hw/arm/omap2.c | 1 - hw/arm/omap_sx1.c| 1 - hw/arm/pxa2xx.c | 1 - hw/arm/spitz.c | 1 - hw/arm/versatilepb.c | 1 - hw/arm/vexpress.c| 1 - hw/arm/virt.c| 1 - hw/arm/xilinx_zynq.c | 1 - hw/arm/z2.c | 1 - hw/block/dataplane/virtio-blk.c | 1 - hw/block/virtio-blk.c| 1 - hw/core/qdev-properties.c| 1 - hw/cris/axis_dev88.c | 1 - hw/display/tc6393xb.c| 1 - hw/ide/pci.c | 1 - hw/ide/via.c | 1 - hw/isa/isa-superio.c | 1 - hw/lm32/lm32_boards.c| 1 - hw/lm32/milkymist.c | 1 - hw/microblaze/petalogix_ml605_mmu.c | 1 - hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 - hw/mips/mips_r4k.c | 1 - hw/ppc/spapr.c | 1 - hw/ppc/virtex_ml507.c| 2 -- hw/s390x/virtio-ccw.c| 1 - hw/scsi/mptsas.c | 1 - hw/sd/pl181.c| 1 - hw/sd/sdhci.c| 1 - hw/sd/ssi-sd.c | 1 - hw/sh4/r2d.c | 1 - hw/virtio/virtio-pci.c | 1 - hw/xen/xen_devconfig.c | 1 - hw/xtensa/xtfpga.c | 1 - 38 files changed, 39 deletions(-) diff --git a/hw/arm/collie.c b/hw/arm/collie.c index f8c566e2e5..48b732c176 100644 --- a/hw/arm/collie.c +++ b/hw/arm/collie.c @@ -16,7 +16,6 @@ #include "strongarm.h" #include "hw/arm/arm.h" #include "hw/block/flash.h" -#include "sysemu/block-backend.h" #include "exec/address-spaces.h" #include "cpu.h" diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c index ea2a3c532d..56cb763c4e 100644 --- a/hw/arm/gumstix.c +++ b/hw/arm/gumstix.c @@ -42,7 +42,6 @@ #include "hw/block/flash.h" #include "hw/devices.h" #include "hw/boards.h" -#include "sysemu/block-backend.h" #include "exec/address-spaces.h" #include "sysemu/qtest.h" #include "cpu.h" diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c index 4215c025fc..0beb5c426b 100644 --- a/hw/arm/mainstone.c +++ b/hw/arm/mainstone.c @@ -21,7 +21,6 @@ #include "hw/devices.h" #include "hw/boards.h" #include "hw/block/flash.h" -#include "sysemu/block-backend.h" #include "hw/sysbus.h" #include "exec/address-spaces.h" #include "sysemu/qtest.h" diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index 32687afced..906b7ca22d 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -35,7 +35,6 @@ #include "hw/hw.h" #include "hw/bt.h" #include "hw/loader.h" -#include "sysemu/block-backend.h" #include "hw/sysbus.h" #include "qemu/log.h" #include "exec/address-spaces.h" diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c index e54c1f8f99..854996c1ac 100644 --- a/hw/arm/omap1.c +++ b/hw/arm/omap1.c @@ -28,7 +28,6 @@ #include "hw/arm/omap.h" #include "sysemu/sysemu.h" #include "hw/arm/soc_dma.h" -#include "sysemu/block-backend.h" #include "sysemu/blockdev.h" #include "sysemu/qtest.h" #include "qemu/range.h" diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c index b8d0910a1f..cc4250b7da 100644 --- a/hw/arm/omap2.c +++ b/hw/arm/omap2.c @@ -23,7 +23,6 @@ #include "qapi/error.h" #include "qemu-common.h" #include "cpu.h" -#include "sysemu/block-backend.h" #include "sysemu/blockdev.h" #include "sysemu/qtest.h" #include "hw/boards.h" diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c index eccc19c77b..84550f0236 100644 --- a/hw/arm/omap_sx1.c +++ b/hw/arm/omap_sx1.c @@ -33,7 +33,6 @@ #include "hw/boards.h" #include "hw/arm/arm.h" #include "hw/block/flash.h" -#include "sysemu/block-backend.h" #include "sysemu/qtest.h" #include "exec/address-spaces.h" #include "cpu.h" diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index a2803fdee4..b67b0cefb6 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -19,7 +19,6 @@ #include "hw/i2c/i2c.h" #include "hw/ssi/ssi.h" #include "chardev/char-fe.h" -#include "sysemu/block-backend.h" #include "sysemu/blockdev.h" #include "sysemu/qtest.h" #include "qemu/cutils.h" diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index e419e3c00e..3cc27a1e44 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -27,7 +27,6 @@ #include "hw/audio/wm8750.h" #include "audio/audio.h" #include "hw/boards.h" -#include "sysemu/block-backend.h" #include "hw/sysbus.h"
[Qemu-devel] [PATCH v4 08/21] target/hppa: Include "qemu/log.h" to use qemu_log()
Since his inception in 61766fe9e2d, this file uses the qemu_log() API from "qemu/log.h". Include it to allow further includes cleanup. Signed-off-by: Philippe Mathieu-Daudé --- target/hppa/int_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c index 787f3d6357..561bf6eb60 100644 --- a/target/hppa/int_helper.c +++ b/target/hppa/int_helper.c @@ -19,6 +19,7 @@ #include "qemu/osdep.h" #include "qemu/main-loop.h" +#include "qemu/log.h" #include "cpu.h" #include "exec/exec-all.h" #include "exec/helper-proto.h" -- 2.17.0
[Qemu-devel] [PATCH v4 21/21] hw: Clean "hw/devices.h" includes
Signed-off-by: Philippe Mathieu-Daudé --- This starts the slow process of getting rid of devices.h... --- include/hw/devices.h | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/include/hw/devices.h b/include/hw/devices.h index 861ddea8af..0e27feb0c2 100644 --- a/include/hw/devices.h +++ b/include/hw/devices.h @@ -1,13 +1,10 @@ #ifndef QEMU_DEVICES_H #define QEMU_DEVICES_H -#include "hw/irq.h" - -/* ??? Not all users of this file can include cpu-common.h. */ -struct MemoryRegion; - /* Devices that have nowhere better to go. */ +#include "hw/hw.h" + /* smc91c111.c */ void smc91c111_init(NICInfo *, uint32_t, qemu_irq); -- 2.17.0
[Qemu-devel] [PATCH v4 14/21] hw: Do not include "sysemu/blockdev.h" if it is not necessary
Remove those unneeded includes to speed up the compilation process a little bit. Code change produced with: $ git grep '#include "sysemu/blockdev.h"' | \ cut -d: -f-1 | \ xargs egrep -L "(BlockInterfaceType|DriveInfo|drive_get|blk_legacy_dinfo|blockdev_mark_auto_del)" | \ xargs sed -i.bak '/#include "sysemu\/blockdev.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- hw/block/m25p80.c | 1 - hw/block/onenand.c | 1 - hw/i386/xen/xen-mapcache.c | 1 - hw/s390x/virtio-ccw.c | 1 - hw/scsi/scsi-generic.c | 1 - hw/sd/sdhci.c | 1 - hw/usb/dev-storage.c | 1 - monitor.c | 1 - 8 files changed, 8 deletions(-) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index b49c8e9caa..a5ccffb4aa 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -24,7 +24,6 @@ #include "qemu/osdep.h" #include "hw/hw.h" #include "sysemu/block-backend.h" -#include "sysemu/blockdev.h" #include "hw/ssi/ssi.h" #include "qemu/bitops.h" #include "qemu/log.h" diff --git a/hw/block/onenand.c b/hw/block/onenand.c index ab0c7ea1b3..0cb8d7fa13 100644 --- a/hw/block/onenand.c +++ b/hw/block/onenand.c @@ -25,7 +25,6 @@ #include "hw/block/flash.h" #include "hw/irq.h" #include "sysemu/block-backend.h" -#include "sysemu/blockdev.h" #include "exec/memory.h" #include "hw/sysbus.h" #include "qemu/error-report.h" diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c index efa35dc6e0..541b7693b3 100644 --- a/hw/i386/xen/xen-mapcache.c +++ b/hw/i386/xen/xen-mapcache.c @@ -14,7 +14,6 @@ #include #include "hw/xen/xen_backend.h" -#include "sysemu/blockdev.h" #include "qemu/bitmap.h" #include diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index b68798ac52..0a9bec484b 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -13,7 +13,6 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "hw/hw.h" -#include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "net/net.h" diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 381f04e339..03bce8ff39 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -17,7 +17,6 @@ #include "qemu/error-report.h" #include "hw/scsi/scsi.h" #include "sysemu/block-backend.h" -#include "sysemu/blockdev.h" #ifdef __linux__ diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index b65403947b..3017e5a95a 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -26,7 +26,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "hw/hw.h" -#include "sysemu/blockdev.h" #include "sysemu/dma.h" #include "qemu/timer.h" #include "qemu/bitops.h" diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index b56c75a73a..d02acda945 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -20,7 +20,6 @@ #include "monitor/monitor.h" #include "sysemu/sysemu.h" #include "sysemu/block-backend.h" -#include "sysemu/blockdev.h" #include "qapi/visitor.h" #include "qemu/cutils.h" diff --git a/monitor.c b/monitor.c index 46814af533..d75cb20815 100644 --- a/monitor.c +++ b/monitor.c @@ -44,7 +44,6 @@ #include "qemu/readline.h" #include "ui/console.h" #include "ui/input.h" -#include "sysemu/blockdev.h" #include "sysemu/block-backend.h" #include "audio/audio.h" #include "disas/disas.h" -- 2.17.0
[Qemu-devel] [PATCH v4 20/21] hw/ide: Remove unused include
There is no need to include pci.h in this file. (Continue f23c81073a cleanup). Signed-off-by: Philippe Mathieu-Daudé --- hw/ide/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 866c659498..cc9ca28c33 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -25,7 +25,6 @@ #include "qemu/osdep.h" #include "hw/hw.h" -#include "hw/pci/pci.h" #include "hw/isa/isa.h" #include "qemu/error-report.h" #include "qemu/timer.h" -- 2.17.0
[Qemu-devel] [PATCH v4 17/21] hw/misc/sga: Use the correct ISA include
The SGA BIOS loader is an ISA device, it does not require the PCI header. Signed-off-by: Philippe Mathieu-Daudé --- hw/misc/sga.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/sga.c b/hw/misc/sga.c index 97fd63f176..4a22a52a60 100644 --- a/hw/misc/sga.c +++ b/hw/misc/sga.c @@ -25,7 +25,7 @@ * */ #include "qemu/osdep.h" -#include "hw/pci/pci.h" +#include "hw/isa/isa.h" #include "hw/loader.h" #include "sysemu/sysemu.h" -- 2.17.0
[Qemu-devel] [PATCH v4 19/21] hw/i386/pc: Remove unused include
Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/pc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d768930d02..8b0803cb83 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -64,7 +64,6 @@ #include "hw/acpi/acpi.h" #include "hw/acpi/cpu_hotplug.h" #include "hw/boards.h" -#include "hw/pci/pci_host.h" #include "acpi-build.h" #include "hw/mem/pc-dimm.h" #include "qapi/error.h" -- 2.17.0
Re: [Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
On Mon, 28 May 2018 20:26:59 -0300 Philippe Mathieu-Daudé wrote: -ENOCOMMITLOG Why? Tangible benefit. Looks like noise. Thanks, Alex > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/vfio/vfio-common.h | 1 - > hw/vfio/ccw.c | 1 + > hw/vfio/platform.c| 1 + > 3 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index d9360148e6..8264a65fa5 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -22,7 +22,6 @@ > #define HW_VFIO_VFIO_COMMON_H > > #include "qemu-common.h" > -#include "exec/address-spaces.h" > #include "exec/memory.h" > #include "qemu/queue.h" > #include "qemu/notify.h" > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index e67392c5f9..76e4e8c652 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -22,6 +22,7 @@ > #include "hw/vfio/vfio-common.h" > #include "hw/s390x/s390-ccw.h" > #include "hw/s390x/ccw-device.h" > +#include "exec/address-spaces.h" > #include "qemu/error-report.h" > > #define TYPE_VFIO_CCW "vfio-ccw" > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > index 5c921c27ba..57c4a0ee2b 100644 > --- a/hw/vfio/platform.c > +++ b/hw/vfio/platform.c > @@ -24,6 +24,7 @@ > #include "qemu/range.h" > #include "sysemu/sysemu.h" > #include "exec/memory.h" > +#include "exec/address-spaces.h" > #include "qemu/queue.h" > #include "hw/sysbus.h" > #include "trace.h"
[Qemu-devel] [PATCH v4 18/21] hw/hppa: Remove unused include
Signed-off-by: Philippe Mathieu-Daudé --- hw/hppa/hppa_sys.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/hppa/hppa_sys.h b/hw/hppa/hppa_sys.h index a182d1f34e..3f6c145120 100644 --- a/hw/hppa/hppa_sys.h +++ b/hw/hppa/hppa_sys.h @@ -3,7 +3,6 @@ #ifndef HW_HPPA_SYS_H #define HW_HPPA_SYS_H -#include "target/hppa/cpu-qom.h" #include "hw/pci/pci.h" #include "hw/pci/pci_host.h" #include "hw/ide.h" -- 2.17.0
[Qemu-devel] [PATCH v4 15/21] hw/block/nvme: Include "qemu/cutils.h" directly in the source file
Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth --- hw/block/nvme.h | 1 - hw/block/nvme.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.h b/hw/block/nvme.h index 8f3981121d..cabcf20c32 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -1,6 +1,5 @@ #ifndef HW_NVME_H #define HW_NVME_H -#include "qemu/cutils.h" #include "block/nvme.h" typedef struct NvmeAsyncEvent { diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 85d2406400..811084b6a7 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -35,6 +35,7 @@ #include "sysemu/block-backend.h" #include "qemu/log.h" +#include "qemu/cutils.h" #include "trace.h" #include "nvme.h" -- 2.17.0
[Qemu-devel] [PATCH v4 09/21] target: Do not include "exec/exec-all.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/exec-all.h"' | \ cut -d: -f-1 | \ xargs egrep -L "(cpu_address_space_init|cpu_loop_|tlb_|tb_|GETPC|singlestep|TranslationBlock)" | \ xargs sed -i.bak '/#include "exec\/exec-all.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- bsd-user/qemu.h| 1 - target/arm/arm_ldst.h | 1 - hw/i386/kvmvapic.c | 1 - target/arm/arm-powerctl.c | 1 - target/arm/crypto_helper.c | 1 - target/arm/iwmmxt_helper.c | 1 - target/arm/neon_helper.c | 1 - target/arm/psci.c | 1 - target/arm/vec_helper.c| 1 - target/cris/cpu.c | 1 - target/hppa/helper.c | 1 - target/hppa/int_helper.c | 1 - target/i386/hax-all.c | 1 - target/i386/hax-mem.c | 1 - target/i386/hax-windows.c | 1 - target/i386/hvf/hvf.c | 1 - target/i386/hvf/x86_task.c | 1 - target/i386/whpx-all.c | 1 - target/lm32/cpu.c | 1 - target/m68k/cpu.c | 1 - target/moxie/cpu.c | 1 - target/moxie/mmu.c | 1 - target/openrisc/cpu.c | 1 - target/ppc/int_helper.c| 1 - target/s390x/cpu.c | 1 - target/s390x/diag.c| 1 - target/s390x/helper.c | 1 - target/tilegx/cpu.c| 1 - target/xtensa/core-dc232b.c| 1 - target/xtensa/core-dc233c.c| 1 - target/xtensa/core-de212.c | 1 - target/xtensa/core-fsf.c | 1 - target/xtensa/core-sample_controller.c | 1 - target/xtensa/cpu.c| 1 - tcg/tcg-op-vec.c | 1 - target/xtensa/import_core.sh | 1 - 36 files changed, 36 deletions(-) diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index 19b2b8fecb..09e8aed9c7 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -19,7 +19,6 @@ #include "cpu.h" -#include "exec/exec-all.h" #include "exec/cpu_ldst.h" #undef DEBUG_REMAP diff --git a/target/arm/arm_ldst.h b/target/arm/arm_ldst.h index 01587b3ebb..5e0ac8bef0 100644 --- a/target/arm/arm_ldst.h +++ b/target/arm/arm_ldst.h @@ -20,7 +20,6 @@ #ifndef ARM_LDST_H #define ARM_LDST_H -#include "exec/exec-all.h" #include "exec/cpu_ldst.h" #include "qemu/bswap.h" diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index fc962c5fbc..70f6f26a94 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -11,7 +11,6 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "cpu.h" -#include "exec/exec-all.h" #include "sysemu/sysemu.h" #include "sysemu/cpus.h" #include "sysemu/hw_accel.h" diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c index 25207cb850..ce55eeb682 100644 --- a/target/arm/arm-powerctl.c +++ b/target/arm/arm-powerctl.c @@ -15,7 +15,6 @@ #include "arm-powerctl.h" #include "qemu/log.h" #include "qemu/main-loop.h" -#include "exec/exec-all.h" #ifndef DEBUG_ARM_POWERCTL #define DEBUG_ARM_POWERCTL 0 diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c index cc339ea7e0..f800266727 100644 --- a/target/arm/crypto_helper.c +++ b/target/arm/crypto_helper.c @@ -12,7 +12,6 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/exec-all.h" #include "exec/helper-proto.h" #include "crypto/aes.h" diff --git a/target/arm/iwmmxt_helper.c b/target/arm/iwmmxt_helper.c index 7d87e1a0a8..f6a4fc5b7f 100644 --- a/target/arm/iwmmxt_helper.c +++ b/target/arm/iwmmxt_helper.c @@ -22,7 +22,6 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/exec-all.h" #include "exec/helper-proto.h" /* iwMMXt macros extracted from GNU gdb. */ diff --git a/target/arm/neon_helper.c b/target/arm/neon_helper.c index a1ec6537eb..c2c6491a83 100644 --- a/target/arm/neon_helper.c +++ b/target/arm/neon_helper.c @@ -9,7 +9,6 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/exec-all.h" #include "exec/helper-proto.h" #include "fpu/softfloat.h" diff --git a/target/arm/psci.c b/target/arm/psci.c index eb7b88e926..a74d78802a 100644 --- a/target/arm/psci.c +++ b/target/arm/psci.c @@ -22,7 +22,6 @@ #include "sysemu/sysemu.h" #include "internals.h" #include "arm-powerctl.h" -#include "exec/exec-all.h" bool arm_is_psci_call(ARMCPU *cpu, int excp_type) { diff --git a/target/arm/vec_helper.c b/target/arm/vec_helper.c index ec705cfca5..25e209da31 100644 --- a/target/arm/vec_helper.c +++ b/target/arm/vec_helper.c @@ -19,7 +19,6 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/exec-all.h" #include "exec/helper-proto.h" #include "tcg/tcg-gvec-desc.h" #include "fpu/softfloat.h" diff --git a/target/cris/cpu.c b/target/cris/cpu.c index db8d0884a1..a23aba2688 100644 --- a/target/cris/cpu.c +++ b/target/cris/cpu.c @@
[Qemu-devel] [PATCH v4 10/21] hw: Do not include "exec/ioport.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/ioport.h"' hw | \ cut -d: -f-1 | \ xargs egrep -Li "(portio|cpu_(in|out).\()" | \ xargs sed -i.bak '/#include "exec\/ioport.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- hw/acpi/pcihp.c | 1 - hw/acpi/piix4.c | 1 - 2 files changed, 2 deletions(-) diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index 91c82fdc7a..80d42e12ff 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -32,7 +32,6 @@ #include "hw/pci/pci.h" #include "hw/acpi/acpi.h" #include "sysemu/sysemu.h" -#include "exec/ioport.h" #include "exec/address-spaces.h" #include "hw/pci/pci_bus.h" #include "qapi/error.h" diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 8b703455b7..6404af5f33 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -28,7 +28,6 @@ #include "sysemu/sysemu.h" #include "qapi/error.h" #include "qemu/range.h" -#include "exec/ioport.h" #include "hw/nvram/fw_cfg.h" #include "exec/address-spaces.h" #include "hw/acpi/piix4.h" -- 2.17.0
[Qemu-devel] [PATCH v4 13/21] hw: Do not include "sysemu/blockdev.h" if it is not necessary
The header "hw/boards.h" already includes "sysemu/blockdev.h". Code change produced with: $ git grep '#include "sysemu/blockdev.h"' hw | \ cut -d: -f-1 | \ xargs fgrep -l '#include "hw/boards.h"' | \ xargs sed -i.bak '/#include "sysemu\/blockdev.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/aspeed.c| 1 - hw/arm/omap1.c | 1 - hw/arm/omap2.c | 1 - hw/mips/mips_malta.c | 1 - hw/ppc/ppc405_boards.c | 1 - hw/ppc/sam460ex.c | 1 - 6 files changed, 6 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index aecb3c1e75..a7110a712f 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -19,7 +19,6 @@ #include "hw/boards.h" #include "qemu/log.h" #include "sysemu/block-backend.h" -#include "sysemu/blockdev.h" #include "hw/loader.h" #include "qemu/error-report.h" diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c index 854996c1ac..9af04728e3 100644 --- a/hw/arm/omap1.c +++ b/hw/arm/omap1.c @@ -28,7 +28,6 @@ #include "hw/arm/omap.h" #include "sysemu/sysemu.h" #include "hw/arm/soc_dma.h" -#include "sysemu/blockdev.h" #include "sysemu/qtest.h" #include "qemu/range.h" #include "hw/sysbus.h" diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c index cc4250b7da..3c7d1364a9 100644 --- a/hw/arm/omap2.c +++ b/hw/arm/omap2.c @@ -23,7 +23,6 @@ #include "qapi/error.h" #include "qemu-common.h" #include "cpu.h" -#include "sysemu/blockdev.h" #include "sysemu/qtest.h" #include "hw/boards.h" #include "hw/hw.h" diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index af70ecffc0..494f84e290 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -46,7 +46,6 @@ #include "elf.h" #include "hw/timer/mc146818rtc.h" #include "hw/timer/i8254.h" -#include "sysemu/blockdev.h" #include "exec/address-spaces.h" #include "hw/sysbus.h" /* SysBusDevice */ #include "qemu/host-utils.h" diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index 0b658931ee..d301067d3b 100644 --- a/hw/ppc/ppc405_boards.c +++ b/hw/ppc/ppc405_boards.c @@ -37,7 +37,6 @@ #include "qemu/log.h" #include "qemu/error-report.h" #include "hw/loader.h" -#include "sysemu/blockdev.h" #include "exec/address-spaces.h" #define BIOS_FILENAME "ppc405_rom.bin" diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index a48e6e6fce..3dd23de71f 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -17,7 +17,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "hw/hw.h" -#include "sysemu/blockdev.h" #include "hw/boards.h" #include "sysemu/kvm.h" #include "kvm_ppc.h" -- 2.17.0
[Qemu-devel] [PATCH v4 07/21] target/ppc: Include "exec/exec-all.h" which provides tlb_flush()
Since it inception this include uses tlb_flush() declared in "exec/exec-all.h". Include the other header to allow further includes cleanup. Signed-off-by: Philippe Mathieu-Daudé --- target/ppc/helper_regs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h index 84fd30c2db..5efd18049e 100644 --- a/target/ppc/helper_regs.h +++ b/target/ppc/helper_regs.h @@ -21,6 +21,7 @@ #define HELPER_REGS_H #include "qemu/main-loop.h" +#include "exec/exec-all.h" /* Swap temporary saved registers with GPRs */ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env) -- 2.17.0
[Qemu-devel] [PATCH v4 06/21] target/xtensa: Include "qemu/timer.h" to use NANOSECONDS_PER_SECOND
Since d0ce7e9cfc the dc232b structure uses the NANOSECONDS_PER_SECOND definition from "qemu/timer.h". Include it to allow further includes cleanup. Signed-off-by: Philippe Mathieu-Daudé --- target/xtensa/core-dc232b.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/xtensa/core-dc232b.c b/target/xtensa/core-dc232b.c index 7331eeea2f..70f33622ec 100644 --- a/target/xtensa/core-dc232b.c +++ b/target/xtensa/core-dc232b.c @@ -30,6 +30,7 @@ #include "exec/exec-all.h" #include "exec/gdbstub.h" #include "qemu/host-utils.h" +#include "qemu/timer.h" #include "core-dc232b/core-isa.h" #include "overlay_tool.h" -- 2.17.0
[Qemu-devel] [PATCH v4 16/21] hw/misc/mips_itu: Cleanup includes
Signed-off-by: Philippe Mathieu-Daudé --- include/hw/misc/mips_itu.h | 2 ++ hw/misc/mips_itu.c | 5 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/hw/misc/mips_itu.h b/include/hw/misc/mips_itu.h index b3a4532036..030eb4ac62 100644 --- a/include/hw/misc/mips_itu.h +++ b/include/hw/misc/mips_itu.h @@ -20,6 +20,8 @@ #ifndef MIPS_ITU_H #define MIPS_ITU_H +#include "hw/sysbus.h" + #define TYPE_MIPS_ITU "mips-itu" #define MIPS_ITU(obj) OBJECT_CHECK(MIPSITUState, (obj), TYPE_MIPS_ITU) diff --git a/hw/misc/mips_itu.c b/hw/misc/mips_itu.c index c84a48bbb7..ccc4c7d98a 100644 --- a/hw/misc/mips_itu.c +++ b/hw/misc/mips_itu.c @@ -18,13 +18,10 @@ */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "qapi/error.h" #include "cpu.h" -#include "qemu/log.h" #include "exec/exec-all.h" -#include "hw/hw.h" -#include "hw/sysbus.h" -#include "sysemu/sysemu.h" #include "hw/misc/mips_itu.h" #define ITC_TAG_ADDRSPACE_SZ (ITC_ADDRESSMAP_NUM * 8) -- 2.17.0
[Qemu-devel] [PATCH v4 03/21] target: Do not include "exec/address-spaces.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/address-spaces.h"' target | \ cut -d: -f-1 | \ xargs egrep -L "(get_system_|address_space_)" | \ xargs sed -i.bak '/#include "exec\/address-spaces.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- target/i386/hvf/x86_task.c | 1 - target/s390x/kvm.c | 1 - target/s390x/mem_helper.c | 1 - target/s390x/misc_helper.c | 1 - target/sparc/mmu_helper.c | 1 - 5 files changed, 5 deletions(-) diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c index 4abf3db25e..c3ead2ca73 100644 --- a/target/i386/hvf/x86_task.c +++ b/target/i386/hvf/x86_task.c @@ -26,7 +26,6 @@ #include #include -#include "exec/address-spaces.h" #include "exec/exec-all.h" #include "exec/ioport.h" #include "hw/i386/apic_internal.h" diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 58e4380ae3..ac370da281 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -39,7 +39,6 @@ #include "hw/hw.h" #include "sysemu/device_tree.h" #include "exec/gdbstub.h" -#include "exec/address-spaces.h" #include "trace.h" #include "hw/s390x/s390-pci-inst.h" #include "hw/s390x/s390-pci-bus.h" diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c index a0e28bd124..e21a47fb4d 100644 --- a/target/s390x/mem_helper.c +++ b/target/s390x/mem_helper.c @@ -21,7 +21,6 @@ #include "qemu/osdep.h" #include "cpu.h" #include "internal.h" -#include "exec/address-spaces.h" #include "exec/helper-proto.h" #include "exec/exec-all.h" #include "exec/cpu_ldst.h" diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 1f834f35ef..de1ced2082 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -26,7 +26,6 @@ #include "qemu/host-utils.h" #include "exec/helper-proto.h" #include "qemu/timer.h" -#include "exec/address-spaces.h" #include "exec/exec-all.h" #include "exec/cpu_ldst.h" diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c index f8886ae039..135a9c9d9b 100644 --- a/target/sparc/mmu_helper.c +++ b/target/sparc/mmu_helper.c @@ -21,7 +21,6 @@ #include "cpu.h" #include "exec/exec-all.h" #include "trace.h" -#include "exec/address-spaces.h" /* Sparc MMU emulation */ -- 2.17.0
[Qemu-devel] [PATCH v4 11/21] hw: Do not include "exec/address-spaces.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/address-spaces.h"' hw include/hw | \ cut -d: -f-1 | \ xargs egrep -L "(get_system_|address_space_)" | \ xargs sed -i.bak '/#include "exec\/address-spaces.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- include/hw/arm/allwinner-a10.h | 1 - include/hw/arm/bcm2835_peripherals.h | 1 - include/hw/display/bcm2835_fb.h | 1 - include/hw/dma/bcm2835_dma.h | 1 - include/hw/misc/bcm2835_mbox.h | 1 - include/hw/misc/bcm2835_property.h | 1 - include/hw/sh4/sh_intc.h | 1 - include/hw/virtio/virtio-access.h| 1 - hw/arm/bcm2836.c | 1 - hw/arm/xlnx-zcu102.c | 1 - hw/block/onenand.c | 1 - hw/block/pflash_cfi01.c | 1 - hw/block/pflash_cfi02.c | 1 - hw/char/mcf_uart.c | 1 - hw/char/serial.c | 1 - hw/char/sh_serial.c | 1 - hw/core/loader-fit.c | 1 - hw/core/platform-bus.c | 1 - hw/display/sm501.c | 1 - hw/m68k/mcf5206.c| 1 - hw/m68k/mcf_intc.c | 1 - hw/misc/arm_integrator_debug.c | 1 - hw/net/mcf_fec.c | 1 - hw/net/ne2000-isa.c | 1 - hw/pci-host/versatile.c | 1 - hw/riscv/riscv_htif.c| 1 - hw/sh4/sh7750.c | 1 - hw/timer/sh_timer.c | 1 - 28 files changed, 28 deletions(-) diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h index 6b32a99e21..efb8fc8123 100644 --- a/include/hw/arm/allwinner-a10.h +++ b/include/hw/arm/allwinner-a10.h @@ -11,7 +11,6 @@ #include "hw/ide/ahci.h" #include "sysemu/sysemu.h" -#include "exec/address-spaces.h" #define AW_A10_PIC_REG_BASE 0x01c20400 diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h index 122b286de7..f5b193f670 100644 --- a/include/hw/arm/bcm2835_peripherals.h +++ b/include/hw/arm/bcm2835_peripherals.h @@ -12,7 +12,6 @@ #define BCM2835_PERIPHERALS_H #include "qemu-common.h" -#include "exec/address-spaces.h" #include "hw/sysbus.h" #include "hw/char/bcm2835_aux.h" #include "hw/display/bcm2835_fb.h" diff --git a/include/hw/display/bcm2835_fb.h b/include/hw/display/bcm2835_fb.h index 9a12d7afa2..ae0a3807f2 100644 --- a/include/hw/display/bcm2835_fb.h +++ b/include/hw/display/bcm2835_fb.h @@ -12,7 +12,6 @@ #define BCM2835_FB_H #include "hw/sysbus.h" -#include "exec/address-spaces.h" #include "ui/console.h" #define TYPE_BCM2835_FB "bcm2835-fb" diff --git a/include/hw/dma/bcm2835_dma.h b/include/hw/dma/bcm2835_dma.h index 75312e2e17..60138f4d31 100644 --- a/include/hw/dma/bcm2835_dma.h +++ b/include/hw/dma/bcm2835_dma.h @@ -7,7 +7,6 @@ #define BCM2835_DMA_H #include "qemu-common.h" -#include "exec/address-spaces.h" #include "hw/sysbus.h" typedef struct { diff --git a/include/hw/misc/bcm2835_mbox.h b/include/hw/misc/bcm2835_mbox.h index f4e9ff9ef6..7e8f3ce86d 100644 --- a/include/hw/misc/bcm2835_mbox.h +++ b/include/hw/misc/bcm2835_mbox.h @@ -8,7 +8,6 @@ #include "bcm2835_mbox_defs.h" #include "hw/sysbus.h" -#include "exec/address-spaces.h" #define TYPE_BCM2835_MBOX "bcm2835-mbox" #define BCM2835_MBOX(obj) \ diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h index edcab603ce..11be0dbeac 100644 --- a/include/hw/misc/bcm2835_property.h +++ b/include/hw/misc/bcm2835_property.h @@ -7,7 +7,6 @@ #define BCM2835_PROPERTY_H #include "hw/sysbus.h" -#include "exec/address-spaces.h" #include "net/net.h" #include "hw/display/bcm2835_fb.h" diff --git a/include/hw/sh4/sh_intc.h b/include/hw/sh4/sh_intc.h index 7913bc48a2..fbcee94ed7 100644 --- a/include/hw/sh4/sh_intc.h +++ b/include/hw/sh4/sh_intc.h @@ -3,7 +3,6 @@ #include "qemu-common.h" #include "hw/irq.h" -#include "exec/address-spaces.h" typedef unsigned char intc_enum; diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h index 2e92074bd1..bdf58f3119 100644 --- a/include/hw/virtio/virtio-access.h +++ b/include/hw/virtio/virtio-access.h @@ -18,7 +18,6 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-bus.h" -#include "exec/address-spaces.h" #if defined(TARGET_PPC64) || defined(TARGET_ARM) #define LEGACY_VIRTIO_IS_BIENDIAN 1 diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index 3c4b44a53e..6805a7d7c8 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -15,7 +15,6 @@ #include "hw/arm/bcm2836.h" #include "hw/arm/raspi_platform.h" #include "hw/sysbus.h" -#include "exec/address-spaces.h" /* Peripheral base address seen by the CPU */ #define BCM2836_PERI_BASE 0x3F00 diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index b126cf148b..c70278c8c1 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -22,7 +22,6 @@ #include "hw/arm/xlnx-zynqmp.h"
[Qemu-devel] [PATCH v4 05/21] target/i386: Do not include "exec/ioport.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/ioport.h"' target | \ cut -d: -f-1 | \ xargs egrep -Li "(portio|cpu_(in|out).\()" | \ xargs sed -i.bak '/#include "exec\/ioport.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- target/i386/hax-all.c | 1 - target/i386/hvf/hvf.c | 1 - target/i386/hvf/x86_task.c | 1 - target/i386/kvm.c | 1 - 4 files changed, 4 deletions(-) diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index cad7531406..c5856bbdc3 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -27,7 +27,6 @@ #include "cpu.h" #include "exec/address-spaces.h" #include "exec/exec-all.h" -#include "exec/ioport.h" #include "qemu-common.h" #include "hax-i386.h" diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c index c36753954b..f6c872e678 100644 --- a/target/i386/hvf/hvf.c +++ b/target/i386/hvf/hvf.c @@ -66,7 +66,6 @@ #include "exec/address-spaces.h" #include "exec/exec-all.h" -#include "exec/ioport.h" #include "hw/i386/apic_internal.h" #include "hw/boards.h" #include "qemu/main-loop.h" diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c index c3ead2ca73..7610d85802 100644 --- a/target/i386/hvf/x86_task.c +++ b/target/i386/hvf/x86_task.c @@ -27,7 +27,6 @@ #include #include "exec/exec-all.h" -#include "exec/ioport.h" #include "hw/i386/apic_internal.h" #include "hw/boards.h" #include "qemu/main-loop.h" diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 6511329d11..9d8f80f4c0 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -40,7 +40,6 @@ #include "hw/i386/intel_iommu.h" #include "hw/i386/x86-iommu.h" -#include "exec/ioport.h" #include "hw/pci/pci.h" #include "hw/pci/msi.h" #include "hw/pci/msix.h" -- 2.17.0
[Qemu-devel] [PATCH v4 04/21] memory: Do not include "exec/ioport.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/ioport.h"' memory.c | \ cut -d: -f-1 | \ xargs egrep -Li "(portio|cpu_(in|out).\()" | \ xargs sed -i.bak '/#include "exec\/ioport.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- memory.c | 1 - 1 file changed, 1 deletion(-) diff --git a/memory.c b/memory.c index fc7f9b782b..7ead90f8cd 100644 --- a/memory.c +++ b/memory.c @@ -19,7 +19,6 @@ #include "cpu.h" #include "exec/memory.h" #include "exec/address-spaces.h" -#include "exec/ioport.h" #include "qapi/visitor.h" #include "qemu/bitops.h" #include "qemu/error-report.h" -- 2.17.0
[Qemu-devel] [PATCH v4 00/21] Includes cleanup
Hi, I split the previous series "Use the BYTE-based definitions when useful", this is the first generic part, only headers cleanup, which is big enough. Many patches, but "12 insertions(+), 145 deletions(-)" \o/ v3 was: http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02049.html Philippe Mathieu-Daudé (21): vfio: Include "exec/address-spaces.h" directly in the source file accel: Do not include "exec/address-spaces.h" if it is not necessary target: Do not include "exec/address-spaces.h" if it is not necessary memory: Do not include "exec/ioport.h" if it is not necessary target/i386: Do not include "exec/ioport.h" if it is not necessary target/xtensa: Include "qemu/timer.h" to use NANOSECONDS_PER_SECOND target/ppc: Include "exec/exec-all.h" which provides tlb_flush() target/hppa: Include "qemu/log.h" to use qemu_log() target: Do not include "exec/exec-all.h" if it is not necessary hw: Do not include "exec/ioport.h" if it is not necessary hw: Do not include "exec/address-spaces.h" if it is not necessary hw: Do not include "sysemu/block-backend.h" if it is not necessary hw: Do not include "sysemu/blockdev.h" if it is not necessary hw: Do not include "sysemu/blockdev.h" if it is not necessary hw/block/nvme: Include "qemu/cutils.h" directly in the source file hw/misc/mips_itu: Cleanup includes hw/misc/sga: Use the correct ISA include hw/hppa: Remove unused include hw/i386/pc: Remove unused include hw/ide: Remove unused include hw: Clean "hw/devices.h" includes bsd-user/qemu.h | 1 - hw/block/nvme.h | 1 - hw/hppa/hppa_sys.h | 1 - include/hw/arm/allwinner-a10.h | 1 - include/hw/arm/bcm2835_peripherals.h | 1 - include/hw/devices.h | 7 ++- include/hw/display/bcm2835_fb.h | 1 - include/hw/dma/bcm2835_dma.h | 1 - include/hw/misc/bcm2835_mbox.h | 1 - include/hw/misc/bcm2835_property.h | 1 - include/hw/misc/mips_itu.h | 2 ++ include/hw/sh4/sh_intc.h | 1 - include/hw/vfio/vfio-common.h| 1 - include/hw/virtio/virtio-access.h| 1 - target/arm/arm_ldst.h| 1 - target/ppc/helper_regs.h | 1 + accel/tcg/cpu-exec.c | 1 - hw/acpi/pcihp.c | 1 - hw/acpi/piix4.c | 1 - hw/arm/aspeed.c | 1 - hw/arm/bcm2836.c | 1 - hw/arm/collie.c | 1 - hw/arm/gumstix.c | 1 - hw/arm/mainstone.c | 1 - hw/arm/nseries.c | 1 - hw/arm/omap1.c | 2 -- hw/arm/omap2.c | 2 -- hw/arm/omap_sx1.c| 1 - hw/arm/pxa2xx.c | 1 - hw/arm/spitz.c | 1 - hw/arm/versatilepb.c | 1 - hw/arm/vexpress.c| 1 - hw/arm/virt.c| 1 - hw/arm/xilinx_zynq.c | 1 - hw/arm/xlnx-zcu102.c | 1 - hw/arm/z2.c | 1 - hw/block/dataplane/virtio-blk.c | 1 - hw/block/m25p80.c| 1 - hw/block/nvme.c | 1 + hw/block/onenand.c | 2 -- hw/block/pflash_cfi01.c | 1 - hw/block/pflash_cfi02.c | 1 - hw/block/virtio-blk.c| 1 - hw/char/mcf_uart.c | 1 - hw/char/serial.c | 1 - hw/char/sh_serial.c | 1 - hw/core/loader-fit.c | 1 - hw/core/platform-bus.c | 1 - hw/core/qdev-properties.c| 1 - hw/cris/axis_dev88.c | 1 - hw/display/sm501.c | 1 - hw/display/tc6393xb.c| 1 - hw/i386/kvmvapic.c | 1 - hw/i386/pc.c | 1 - hw/i386/xen/xen-mapcache.c | 1 - hw/ide/core.c| 1 - hw/ide/pci.c | 1 - hw/ide/via.c | 1 - hw/isa/isa-superio.c | 1 - hw/lm32/lm32_boards.c| 1 - hw/lm32/milkymist.c | 1 - hw/m68k/mcf5206.c| 1 - hw/m68k/mcf_intc.c | 1 - hw/microblaze/petalogix_ml605_mmu.c | 1 - hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 - hw/mips/mips_malta.c | 1 - hw/mips/mips_r4k.c | 1 - hw/misc/arm_integrator_debug.c | 1 - hw/misc/mips_itu.c | 5 + hw/misc/sga.c| 2 +- hw/net/mcf_fec.c | 1 - hw/net/ne2000-isa.c
[Qemu-devel] [PATCH v4 01/21] vfio: Include "exec/address-spaces.h" directly in the source file
Signed-off-by: Philippe Mathieu-Daudé --- include/hw/vfio/vfio-common.h | 1 - hw/vfio/ccw.c | 1 + hw/vfio/platform.c| 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index d9360148e6..8264a65fa5 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -22,7 +22,6 @@ #define HW_VFIO_VFIO_COMMON_H #include "qemu-common.h" -#include "exec/address-spaces.h" #include "exec/memory.h" #include "qemu/queue.h" #include "qemu/notify.h" diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index e67392c5f9..76e4e8c652 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -22,6 +22,7 @@ #include "hw/vfio/vfio-common.h" #include "hw/s390x/s390-ccw.h" #include "hw/s390x/ccw-device.h" +#include "exec/address-spaces.h" #include "qemu/error-report.h" #define TYPE_VFIO_CCW "vfio-ccw" diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index 5c921c27ba..57c4a0ee2b 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -24,6 +24,7 @@ #include "qemu/range.h" #include "sysemu/sysemu.h" #include "exec/memory.h" +#include "exec/address-spaces.h" #include "qemu/queue.h" #include "hw/sysbus.h" #include "trace.h" -- 2.17.0
[Qemu-devel] [PATCH v4 02/21] accel: Do not include "exec/address-spaces.h" if it is not necessary
Code change produced with: $ git grep '#include "exec/address-spaces.h"' accel | \ cut -d: -f-1 | \ xargs egrep -L "(get_system_|address_space_)" | \ xargs sed -i.bak '/#include "exec\/address-spaces.h"/d' Signed-off-by: Philippe Mathieu-Daudé --- accel/tcg/cpu-exec.c | 1 - 1 file changed, 1 deletion(-) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 0b154cc678..4ef95d8dd3 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -25,7 +25,6 @@ #include "qemu/atomic.h" #include "sysemu/qtest.h" #include "qemu/timer.h" -#include "exec/address-spaces.h" #include "qemu/rcu.h" #include "exec/tb-hash.h" #include "exec/tb-lookup.h" -- 2.17.0
Re: [Qemu-devel] storing machine data in qcow images?
On Mon, May 28, 2018 at 08:38:33PM +0200, Kevin Wolf wrote: > Just accessing the image file within a tar archive is possible and we > could write a block driver for that (I actually think we should do > this), but it restricts you because certain operations like resizing > aren't really possible in tar. Unfortunately, resizing is a really > common operation for non-raw image formats. We do this already in virt-v2v (using file.offset and file.size parameters in the raw driver). For virt-v2v we only need to read the source so resizing isn't an issue. For most of the cases we're talking about the downloaded image would also be a template / base image, so I suppose only reading would be required too. I also wrote an nbdkit tar file driver (supports writes, but not resizing). https://manpages.debian.org/testing/nbdkit-plugin-perl/nbdkit-tar-plugin.1.en.html Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [Qemu-devel] storing machine data in qcow images?
On Mon, May 28, 2018 at 10:20:54PM +0100, Richard W.M. Jones wrote: > On Mon, May 28, 2018 at 08:38:33PM +0200, Kevin Wolf wrote: > > Just accessing the image file within a tar archive is possible and we > > could write a block driver for that (I actually think we should do > > this), but it restricts you because certain operations like resizing > > aren't really possible in tar. Unfortunately, resizing is a really > > common operation for non-raw image formats. > > We do this already in virt-v2v (using file.offset and file.size > parameters in the raw driver). > > For virt-v2v we only need to read the source so resizing isn't an > issue. For most of the cases we're talking about the downloaded image > would also be a template / base image, so I suppose only reading would > be required too. > > I also wrote an nbdkit tar file driver (supports writes, but not > resizing). > https://manpages.debian.org/testing/nbdkit-plugin-perl/nbdkit-tar-plugin.1.en.html I should add the other thorny issue with OVA files is that the metadata contains a checksum (SHA1 or SHA256) of the disk images. If you modify the disk images in-place in the tar file then you need to recalculate those. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [Qemu-devel] [PATCH] linux-user: SPARC "rd %tick" can be used by user application
On 05/28/2018 04:48 PM, Laurent Vivier wrote: > we have the same problem decribed in 7d6b1daedd > ("linux-user, ppc: mftbl can be used by user application") > for ppc in the case of sparc. > > When we use an application trying to resolve a name, it hangs in > > 0xff5dd40c: rd %tick, %o5 > 0xff5dd410: srlx %o5, 0x20, %o4 > 0xff5dd414: btst %o5, %g4 > 0xff5dd418: be %icc, 0xff5dd40c > > because %tick is staying at 0. > > As QEMU_CLOCK_VIRTUAL is not available in linux-user mode, > simply use cpu_get_host_ticks() instead. > > Signed-off-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé > --- > target/sparc/helper.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target/sparc/helper.c b/target/sparc/helper.c > index 1d854890b4..46232788c8 100644 > --- a/target/sparc/helper.c > +++ b/target/sparc/helper.c > @@ -67,7 +67,9 @@ uint64_t helper_tick_get_count(CPUSPARCState *env, void > *opaque, int mem_idx) > > return cpu_tick_get_count(timer); > #else > -return 0; > +/* In user-mode, QEMU_CLOCK_VIRTUAL doesn't exist. > + Just pass through the host cpu clock ticks. */ > +return cpu_get_host_ticks(); > #endif > } > >
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Use proper logging function for possible guest errors
On Mon, 28 May 2018 20:11:19 +0200 Thomas Huth wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth > --- Reviewed-by: Greg Kurz > target/ppc/translate.c | 37 - > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > -fprintf(stderr, "Trying to read privileged spr %d (0x%03x) > at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged > spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, > sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > -fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > -fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > -if (qemu_log_separate()) { > -qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > -fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10,
[Qemu-devel] [PATCH] linux-user: SPARC "rd %tick" can be used by user application
we have the same problem decribed in 7d6b1daedd ("linux-user, ppc: mftbl can be used by user application") for ppc in the case of sparc. When we use an application trying to resolve a name, it hangs in 0xff5dd40c: rd %tick, %o5 0xff5dd410: srlx %o5, 0x20, %o4 0xff5dd414: btst %o5, %g4 0xff5dd418: be %icc, 0xff5dd40c because %tick is staying at 0. As QEMU_CLOCK_VIRTUAL is not available in linux-user mode, simply use cpu_get_host_ticks() instead. Signed-off-by: Laurent Vivier --- target/sparc/helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/sparc/helper.c b/target/sparc/helper.c index 1d854890b4..46232788c8 100644 --- a/target/sparc/helper.c +++ b/target/sparc/helper.c @@ -67,7 +67,9 @@ uint64_t helper_tick_get_count(CPUSPARCState *env, void *opaque, int mem_idx) return cpu_tick_get_count(timer); #else -return 0; +/* In user-mode, QEMU_CLOCK_VIRTUAL doesn't exist. + Just pass through the host cpu clock ticks. */ +return cpu_get_host_ticks(); #endif } -- 2.14.3
Re: [Qemu-devel] storing machine data in qcow images?
Am 28.05.2018 um 20:44 hat Max Reitz geschrieben: > On 2018-05-28 20:38, Kevin Wolf wrote: > > Am 28.05.2018 um 20:30 hat Richard W.M. Jones geschrieben: > >> On Mon, May 28, 2018 at 08:10:32PM +0200, Max Reitz wrote: > >>> As someone who is just naive and doesn't see the big picture, I don't > >>> see what's wrong with using a tar file that contains the image and > >>> additional data. > >> > >> FWIW an OVA file is exactly this: an uncompressed tar file containing > >> disk image(s) and metadata. > > > > If we combine VM configuration and the disk image this way, I would > > still want to directly use that combined thing without having to extract > > its components first. > > > > Just accessing the image file within a tar archive is possible and we > > could write a block driver for that (I actually think we should do > > this), but it restricts you because certain operations like resizing > > aren't really possible in tar. Unfortunately, resizing is a really > > common operation for non-raw image formats. > > > > And if I think of a file format that can contain several different > > things that can be individually resized etc., I end up with qcow2 in the > > simple case or a full file system in the more complex case. > > Well, you end up with VMDK. I don't think VMDK can save several different objects? It can have some metadata in the descriptor, and it can spread the contents of a single object across multiple files (with extents), but I don't think it has something comparable to e.g. qcow2 snapshots, which are separate objects with an individual size that can dynamically change. Kevin signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors
On 05/28/2018 03:58 PM, Edgar E. Iglesias wrote: > On Mon, May 28, 2018 at 08:48:59PM +0200, Francisco Iglesias wrote: >> Coverity found that the string return by 'object_get_canonical_path' was not >> being freed at two locations in the model (CID 1391294 and CID 1391293) and >> also that a memset was being called with a value greater than the max of a >> byte >> on the second argument (CID 1391286). This patch corrects this by adding the >> freeing of the strings and also changing to memset to zero instead on >> descriptor unaligned errors. > > Perhaps this should have been two patches but in any case: > > Reviewed-by: Edgar E. Iglesias Reviewed-by: Philippe Mathieu-Daudé >> Signed-off-by: Francisco Iglesias >> --- >> hw/dma/xlnx-zdma.c | 10 +++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c >> index 14d86c254b..8eea757aff 100644 >> --- a/hw/dma/xlnx-zdma.c >> +++ b/hw/dma/xlnx-zdma.c >> @@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t >> addr, void *buf) >> qemu_log_mask(LOG_GUEST_ERROR, >>"zdma: unaligned descriptor at %" PRIx64, >>addr); >> -memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr)); >> +memset(buf, 0x0, sizeof(XlnxZDMADescr)); >> s->error = true; >> return false; >> } >> @@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, >> unsigned size) >> RegisterInfo *r = >regs_info[addr / 4]; >> >> if (!r->data) { >> +gchar *path = object_get_canonical_path(OBJECT(s)); >> qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n", >> - object_get_canonical_path(OBJECT(s)), >> + path, >> addr); >> +g_free(path); >> ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); >> zdma_ch_imr_update_irq(s); >> return 0; >> @@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, >> uint64_t value, >> RegisterInfo *r = >regs_info[addr / 4]; >> >> if (!r->data) { >> +gchar *path = object_get_canonical_path(OBJECT(s)); >> qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 >> "\n", >> - object_get_canonical_path(OBJECT(s)), >> + path, >> addr, value); >> +g_free(path); >> ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); >> zdma_ch_imr_update_irq(s); >> return; >> -- >> 2.11.0 >> >
Re: [Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors
On Mon, May 28, 2018 at 08:48:59PM +0200, Francisco Iglesias wrote: > Coverity found that the string return by 'object_get_canonical_path' was not > being freed at two locations in the model (CID 1391294 and CID 1391293) and > also that a memset was being called with a value greater than the max of a > byte > on the second argument (CID 1391286). This patch corrects this by adding the > freeing of the strings and also changing to memset to zero instead on > descriptor unaligned errors. Perhaps this should have been two patches but in any case: Reviewed-by: Edgar E. Iglesias > > Signed-off-by: Francisco Iglesias > --- > hw/dma/xlnx-zdma.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c > index 14d86c254b..8eea757aff 100644 > --- a/hw/dma/xlnx-zdma.c > +++ b/hw/dma/xlnx-zdma.c > @@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t > addr, void *buf) > qemu_log_mask(LOG_GUEST_ERROR, >"zdma: unaligned descriptor at %" PRIx64, >addr); > -memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr)); > +memset(buf, 0x0, sizeof(XlnxZDMADescr)); > s->error = true; > return false; > } > @@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, > unsigned size) > RegisterInfo *r = >regs_info[addr / 4]; > > if (!r->data) { > +gchar *path = object_get_canonical_path(OBJECT(s)); > qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n", > - object_get_canonical_path(OBJECT(s)), > + path, > addr); > +g_free(path); > ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); > zdma_ch_imr_update_irq(s); > return 0; > @@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, > uint64_t value, > RegisterInfo *r = >regs_info[addr / 4]; > > if (!r->data) { > +gchar *path = object_get_canonical_path(OBJECT(s)); > qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n", > - object_get_canonical_path(OBJECT(s)), > + path, > addr, value); > +g_free(path); > ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); > zdma_ch_imr_update_irq(s); > return; > -- > 2.11.0 >
[Qemu-devel] [PATCH v2] linux-user: Remove extra mapping
When a guest mmap()'d a file, a transient MAP_ANONYMOUS mapping was created, which required the kernel to reserve this memory, then subsequently released by applying a mapping with just the requested flags and fd. This transient mapping causes spurious failures when the available memory is smaller than the mapping. This patch avoids this transient mapping. Signed-off-by: Steve Mcpolin --- linux-user/mmap.c | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 9168a20..b33adf2 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -453,21 +453,28 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, /* Note: we prefer to control the mapping address. It is especially important if qemu_host_page_size > qemu_real_host_page_size */ -p = mmap(g2h(start), host_len, prot, - flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0); -if (p == MAP_FAILED) -goto fail; -/* update start so that it points to the file position at 'offset' */ -host_start = (unsigned long)p; -if (!(flags & MAP_ANONYMOUS)) { -p = mmap(g2h(start), len, prot, +if (flags & MAP_ANONYMOUS) { +offset = 0; +host_offset = 0; +fd = -1; +} +p = mmap(g2h(start), len, prot, flags | MAP_FIXED, fd, host_offset); -if (p == MAP_FAILED) { -munmap(g2h(start), host_len); -goto fail; +host_start = (uintptr_t)p; +if (p != MAP_FAILED && host_len > REAL_HOST_PAGE_ALIGN(len)) { +void *q; +q = mmap(g2h(start + len), host_len - REAL_HOST_PAGE_ALIGN(len), + prot, MAP_FIXED | MAP_ANONYMOUS, -1, 0); +if (q == MAP_FAILED) { +p = MAP_FAILED; } -host_start += offset - host_offset; } +if (p == MAP_FAILED) { +munmap(g2h(start), host_len); +goto fail; +} +host_start += offset - host_offset; +/* update start so that it points to the file position at 'offset' */ start = h2g(host_start); } else { if (start & ~TARGET_PAGE_MASK) { -- 2.7.4
Re: [Qemu-devel] storing machine data in qcow images?
On 2018-05-28 20:38, Kevin Wolf wrote: > Am 28.05.2018 um 20:30 hat Richard W.M. Jones geschrieben: >> On Mon, May 28, 2018 at 08:10:32PM +0200, Max Reitz wrote: >>> As someone who is just naive and doesn't see the big picture, I don't >>> see what's wrong with using a tar file that contains the image and >>> additional data. >> >> FWIW an OVA file is exactly this: an uncompressed tar file containing >> disk image(s) and metadata. > > If we combine VM configuration and the disk image this way, I would > still want to directly use that combined thing without having to extract > its components first. > > Just accessing the image file within a tar archive is possible and we > could write a block driver for that (I actually think we should do > this), but it restricts you because certain operations like resizing > aren't really possible in tar. Unfortunately, resizing is a really > common operation for non-raw image formats. > > And if I think of a file format that can contain several different > things that can be individually resized etc., I end up with qcow2 in the > simple case or a full file system in the more complex case. Well, you end up with VMDK. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-trivial] [PATCH] target/ppc: Use proper logging function for possible guest errors
On 05/28/2018 03:11 PM, Thomas Huth wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth Reviewed-by: Philippe Mathieu-Daudé > --- > target/ppc/translate.c | 37 - > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > -fprintf(stderr, "Trying to read privileged spr %d (0x%03x) > at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged > spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, > sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > -fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > -fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > -if (qemu_log_separate()) { > -qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > -fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, >
[Qemu-devel] [PATCH v1] xlnx-zdma: Correct mem leaks and memset to zero on desc unaligned errors
Coverity found that the string return by 'object_get_canonical_path' was not being freed at two locations in the model (CID 1391294 and CID 1391293) and also that a memset was being called with a value greater than the max of a byte on the second argument (CID 1391286). This patch corrects this by adding the freeing of the strings and also changing to memset to zero instead on descriptor unaligned errors. Signed-off-by: Francisco Iglesias --- hw/dma/xlnx-zdma.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c index 14d86c254b..8eea757aff 100644 --- a/hw/dma/xlnx-zdma.c +++ b/hw/dma/xlnx-zdma.c @@ -302,7 +302,7 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr, void *buf) qemu_log_mask(LOG_GUEST_ERROR, "zdma: unaligned descriptor at %" PRIx64, addr); -memset(buf, 0xdeadbeef, sizeof(XlnxZDMADescr)); +memset(buf, 0x0, sizeof(XlnxZDMADescr)); s->error = true; return false; } @@ -707,9 +707,11 @@ static uint64_t zdma_read(void *opaque, hwaddr addr, unsigned size) RegisterInfo *r = >regs_info[addr / 4]; if (!r->data) { +gchar *path = object_get_canonical_path(OBJECT(s)); qemu_log("%s: Decode error: read from %" HWADDR_PRIx "\n", - object_get_canonical_path(OBJECT(s)), + path, addr); +g_free(path); ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); zdma_ch_imr_update_irq(s); return 0; @@ -724,9 +726,11 @@ static void zdma_write(void *opaque, hwaddr addr, uint64_t value, RegisterInfo *r = >regs_info[addr / 4]; if (!r->data) { +gchar *path = object_get_canonical_path(OBJECT(s)); qemu_log("%s: Decode error: write to %" HWADDR_PRIx "=%" PRIx64 "\n", - object_get_canonical_path(OBJECT(s)), + path, addr, value); +g_free(path); ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, INV_APB, true); zdma_ch_imr_update_irq(s); return; -- 2.11.0
Re: [Qemu-devel] storing machine data in qcow images?
On Mon, May 28, 2018 at 08:10:32PM +0200, Max Reitz wrote: > As someone who is just naive and doesn't see the big picture, I don't > see what's wrong with using a tar file that contains the image and > additional data. FWIW an OVA file is exactly this: an uncompressed tar file containing disk image(s) and metadata. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [Qemu-devel] storing machine data in qcow images?
On 2018-05-24 13:32, Richard W.M. Jones wrote: > I read the whole thread and the fundamental problem is that you're > mixing layers. Let qcow2 be a disk image format, and let management > layers deal with metadata and how to run qemu. > > What's going to happen when you have (eg) an OVA file containing qcow2 > files, and the qcow2 files all have different metadata from each other > and from the actual metadata in the OVA? Even the case where you've > got ‘-hda file1.qcow2 -hdb file2.qcow2’ is not properly defined. What > happens if someone uses ‘-M mach1 -hda file.qcow2’ and the machine > type in the qcow2 file conflicts with the command line? > > BTW we have a tooling (libguestfs) which can tell you what devices are > supported by the guest. virt-v2v already uses libguestfs to find out > the full list of devices supported by guests, and uses that to drive > conversion. At some point we're going to extend virt-inspector to > make this a bit easier (patches and other contributions welcome, > there's a huge list of work to do on libguestfs and not enough > developers to get through it). > > There is however a seed of a good idea in the thread: > >> I don't think QEMU needs to use this information automatically, >> necessarily. I think the first step is to simply make QEMU save >> this information in the disk image, and making qemu-img able to >> read and write this information. > > It would be nice if qcow2 added arbitrary data sections (which would > always be ignored by qemu) for storing additional data. This could be > used to create a compact qcow2 + metadata format to rival OVA for > management layers to use, and there are various other uses too. As an extremist on the "qcow2 is an image data format and as such should only store data that is relevant to the virtual disk" front, I don't see the appeal. As someone who is just naive and doesn't see the big picture, I don't see what's wrong with using a tar file that contains the image and additional data. I shudder to imagine integrating the qcow2 driver so deeply into qemu that various parts all over qemu just use it to store some data. I can't help but feel reminded of the HMP savevm command that just randomly chooses some qcow2 image to store the VM state in. At least you're talking about just storing data. I imagine opening a qcow2 image, then reading some VM configuration, and spreading the gospel through the rest of the qemu process to initialize everything would be anything but nice. I personally don't see why it is so bad to split the information between two files. Honestly, if you want to put disk images and VM configuration into a single file, I'd do it backwards: Put the disk image into the configuration file. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] storing machine data in qcow images?
Am 28.05.2018 um 20:30 hat Richard W.M. Jones geschrieben: > On Mon, May 28, 2018 at 08:10:32PM +0200, Max Reitz wrote: > > As someone who is just naive and doesn't see the big picture, I don't > > see what's wrong with using a tar file that contains the image and > > additional data. > > FWIW an OVA file is exactly this: an uncompressed tar file containing > disk image(s) and metadata. If we combine VM configuration and the disk image this way, I would still want to directly use that combined thing without having to extract its components first. Just accessing the image file within a tar archive is possible and we could write a block driver for that (I actually think we should do this), but it restricts you because certain operations like resizing aren't really possible in tar. Unfortunately, resizing is a really common operation for non-raw image formats. And if I think of a file format that can contain several different things that can be individually resized etc., I end up with qcow2 in the simple case or a full file system in the more complex case. Kevin
[Qemu-devel] [PATCH] target/ppc: Use proper logging function for possible guest errors
fprintf() and qemu_log_separate() are frowned upon these days for printing logging information in QEMU. Accessing the wrong SPRs indicates wrong guest behaviour in most cases, and we've got a proper way to log such situations, which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this function now for logging the bad SPR accesses instead. Signed-off-by: Thomas Huth --- target/ppc/translate.c | 37 - 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index e30d99f..0806ee0 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) * allowing userland application to read the PVR */ if (sprn != SPR_PVR) { -fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -if (qemu_log_separate()) { -qemu_log("Trying to read privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, - ctx->base.pc_next - 4); -} +qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, + ctx->base.pc_next - 4); } gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); } @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) return; } /* Not defined */ -fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -if (qemu_log_separate()) { -qemu_log("Trying to read invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -} +qemu_log_mask(LOG_GUEST_ERROR, + "Trying to read invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); /* The behaviour depends on MSR:PR and SPR# bit 0x10, * it can generate a priv, a hv emu or a no-op @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) (*write_cb)(ctx, sprn, rS(ctx->opcode)); } else { /* Privilege exception */ -fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -if (qemu_log_separate()) { -qemu_log("Trying to write privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -} +qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, + ctx->base.pc_next - 4); gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); } } else { @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) } /* Not defined */ -if (qemu_log_separate()) { -qemu_log("Trying to write invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); -} -fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); +qemu_log_mask(LOG_GUEST_ERROR, + "Trying to write invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); /* The behaviour depends on MSR:PR and SPR# bit 0x10, -- 1.8.3.1
Re: [Qemu-devel] [RFC v2 5/6] hw/arm: ACPI SRAT changes to accommodate non-contiguous mem
On Wed, May 16, 2018 at 04:20:25PM +0100, Shameer Kolothum wrote: > This is in preparation for the next patch where initial ram is split > into a non-pluggable chunk and a pc-dimm modeled mem if the vaild > iova regions are non-contiguous. > > Signed-off-by: Shameer Kolothum > --- > hw/arm/virt-acpi-build.c | 24 > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index c7c6a57..8d17b40 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -488,7 +488,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > AcpiSratProcessorGiccAffinity *core; > AcpiSratMemoryAffinity *numamem; > int i, srat_start; > -uint64_t mem_base; > +uint64_t mem_base, mem_sz, mem_len; > MachineClass *mc = MACHINE_GET_CLASS(vms); > const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(MACHINE(vms)); > > @@ -505,12 +505,28 @@ build_srat(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > core->flags = cpu_to_le32(1); > } > > -mem_base = vms->memmap[VIRT_MEM].base; > +mem_base = vms->bootinfo.loader_start; > +mem_sz = vms->bootinfo.loader_start; mem_sz = vms->bootinfo.ram_size; Assuming the DT generator was correct, meaning bootinfo.ram_size will be the size of the non-pluggable dimm. > for (i = 0; i < nb_numa_nodes; ++i) { > numamem = acpi_data_push(table_data, sizeof(*numamem)); > -build_srat_memory(numamem, mem_base, numa_info[i].node_mem, i, > +mem_len = MIN(numa_info[i].node_mem, mem_sz); > +build_srat_memory(numamem, mem_base, mem_len, i, >MEM_AFFINITY_ENABLED); > -mem_base += numa_info[i].node_mem; > +mem_base += mem_len; > +mem_sz -= mem_len; > +if (!mem_sz) { > +break; > +} > +} > + > +/* Create table for initial pc-dimm ram, if any */ > +if (vms->bootinfo.dimm_mem) { > +numamem = acpi_data_push(table_data, sizeof(*numamem)); > +build_srat_memory(numamem, vms->bootinfo.dimm_mem->base, > + vms->bootinfo.dimm_mem->size, > + vms->bootinfo.dimm_mem->node, > + MEM_AFFINITY_ENABLED); > + > } > > build_header(linker, table_data, (void *)(table_data->data + srat_start), > -- > 2.7.4 > > >
Re: [Qemu-devel] [RFC v2 2/6] hw/arm/virt: Enable dynamic generation of guest RAM memory regions
On Wed, May 16, 2018 at 04:20:22PM +0100, Shameer Kolothum wrote: > Register ram_memory_region_init notifier to allocate memory region > from system memory. > > Signed-off-by: Zhu Yijun> Signed-off-by: Shameer Kolothum > --- > hw/arm/virt.c | 28 ++-- > include/hw/arm/virt.h | 1 + > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 94dcb12..05fcb62 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1171,6 +1171,19 @@ void virt_machine_done(Notifier *notifier, void *data) > virt_build_smbios(vms); > } > > +static void virt_ram_memory_region_init(Notifier *notifier, void *data) > +{ > +MemoryRegion *sysmem = get_system_memory(); > +MemoryRegion *ram = g_new(MemoryRegion, 1); > +VirtMachineState *vms = container_of(notifier, VirtMachineState, > + ram_memory_region_init); > +MachineState *machine = MACHINE(vms); > + > +memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram", > + machine->ram_size); > +memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram); > +} > + > static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx) > { > uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; > @@ -1204,7 +1217,6 @@ static void machvirt_init(MachineState *machine) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *secure_sysmem = NULL; > int n, virt_max_cpus; > -MemoryRegion *ram = g_new(MemoryRegion, 1); > bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); > > /* We can probe only here because during property set > @@ -1361,10 +1373,6 @@ static void machvirt_init(MachineState *machine) > fdt_add_timer_nodes(vms); > fdt_add_cpu_nodes(vms); > > -memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram", > - machine->ram_size); > -memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram); > - > create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); > > create_gic(vms, pic); > @@ -1405,15 +1413,23 @@ static void machvirt_init(MachineState *machine) > vms->bootinfo.loader_start = vms->memmap[VIRT_MEM].base; > vms->bootinfo.get_dtb = machvirt_dtb; > vms->bootinfo.firmware_loaded = firmware_loaded; > + > +/* Register notifiers. They are executed in registration reverse order */ > arm_load_kernel(ARM_CPU(first_cpu), >bootinfo); > > /* > * arm_load_kernel machine init done notifier registration must > * happen before the platform_bus_create call. In this latter, > * another notifier is registered which adds platform bus nodes. > - * Notifiers are executed in registration reverse order. > */ > create_platform_bus(vms, pic); > + > +/* > + * Register memory region notifier last as this has to be executed > + * first. > + */ > +vms->ram_memory_region_init.notify = virt_ram_memory_region_init; > +qemu_add_machine_init_done_notifier(>ram_memory_region_init); I realize we've been slow to get to this for reviewing, but a lot has changed here since this patch was written. virt now only has a single machine done notifier. On rebase we should attempt to integrate with that one rather than introduce a new one. Thanks, drew > } > > static bool virt_get_secure(Object *obj, Error **errp) > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index ba0c1a4..fc24f3a 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -91,6 +91,7 @@ typedef struct { > typedef struct { > MachineState parent; > Notifier machine_done; > +Notifier ram_memory_region_init; > FWCfgState *fw_cfg; > bool secure; > bool highmem; > -- > 2.7.4 > > >
Re: [Qemu-devel] [Qemu-arm] [PATCH] ARM: ACPI: Fix use-after-free due to memory realloc
On 05/28/2018 05:42 AM, Shannon Zhao wrote: > acpi_data_push uses g_array_set_size to resize the memory size. If there > is no enough contiguous memory, the address will be changed. So previous > pointer could not be used any more. It must update the pointer and use > the new one. > > Signed-off-by: Shannon Zhao> --- > hw/arm/virt-acpi-build.c | 14 ++ > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 92ceee9..30584ee 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -400,7 +400,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > AcpiIortItsGroup *its; > AcpiIortTable *iort; > AcpiIortSmmu3 *smmu; > -size_t node_size, iort_length, smmu_offset = 0; > +size_t node_size, iort_node_offset, iort_length, smmu_offset = 0; > AcpiIortRC *rc; > > iort = acpi_data_push(table_data, sizeof(*iort)); > @@ -414,6 +414,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > iort_length = sizeof(*iort); > iort->node_count = cpu_to_le32(nb_nodes); > iort->node_offset = cpu_to_le32(sizeof(*iort)); Eventually similar comment to explain why you also use another var: /* * Use a copy in case table_data->data moves during * acpi_data_push operations. */ > +iort_node_offset = iort->node_offset; Reviewed-by: Philippe Mathieu-Daudé > > /* ITS group node */ > node_size = sizeof(*its) + sizeof(uint32_t); > @@ -429,7 +430,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > int irq = vms->irqmap[VIRT_SMMU]; > > /* SMMUv3 node */ > -smmu_offset = iort->node_offset + node_size; > +smmu_offset = iort_node_offset + node_size; > node_size = sizeof(*smmu) + sizeof(*idmap); > iort_length += node_size; > smmu = acpi_data_push(table_data, node_size); > @@ -450,7 +451,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > idmap->id_count = cpu_to_le32(0x); > idmap->output_base = 0; > /* output IORT node is the ITS group node (the first node) */ > -idmap->output_reference = cpu_to_le32(iort->node_offset); > +idmap->output_reference = cpu_to_le32(iort_node_offset); > } > > /* Root Complex Node */ > @@ -479,9 +480,14 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > idmap->output_reference = cpu_to_le32(smmu_offset); > } else { > /* output IORT node is the ITS group node (the first node) */ > -idmap->output_reference = cpu_to_le32(iort->node_offset); > +idmap->output_reference = cpu_to_le32(iort_node_offset); > } > > +/* > + * Update the pointer address in case table_data->data moved during above > + * acpi_data_push operations. > + */ > +iort = (AcpiIortTable *)(table_data->data + iort_start); > iort->length = cpu_to_le32(iort_length); > > build_header(linker, table_data, (void *)(table_data->data + iort_start), >
Re: [Qemu-devel] [PATCH v2] aspeed_scu: Implement RNG register
Hi Joel, On 05/28/2018 12:22 PM, Joel Stanley wrote: > The ASPEED SoCs contain a single register that returns random data when > read. This models that register so that guests can use it. > > The random number data register has a corresponding control register, > data returns a different number regardless of the state of the enabled > bit, so the model follows this behaviour. I have been bugging Cédric to check specs for the RNG_CTRL register and he sent me the v1 of this patch than I missed: >>> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not. >> >> The RNG is enabled by default, and I didn't find any software that >> disables it, but it can't hurt to have that check. > > I did some testing on hardware, and the rng still outputs a different > number each time I ask for one regardless of the state of the enabled > bit. > I confirm that the HW doesn't really care about the enabled bit. Let's ignore it then ? Now your comment makes more sens, however I think it would be more useful to add it in aspeed_scu_read() to make it obvious. > > Signed-off-by: Joel Stanley> --- > v2: > - Remove call to qcrypto_random_init as this is done in main() > --- > hw/misc/aspeed_scu.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 5e6d5744eeca..29e58527793b 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -16,6 +16,7 @@ > #include "qapi/visitor.h" > #include "qemu/bitops.h" > #include "qemu/log.h" > +#include "crypto/random.h" > #include "trace.h" > > #define TO_REG(offset) ((offset) >> 2) > @@ -154,6 +155,18 @@ static const uint32_t > ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { > [BMC_DEV_ID] = 0x2402U > }; > > +static uint32_t aspeed_scu_get_random(void) > +{ > +Error *err = NULL; > +uint32_t num; > + > +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { > +error_report_err(err); > +} > + > +return num; > +} This function duplicates bcm2835_rng::get_random_bytes() except it doesn't exit(), is that on purpose? What about refactoring, inlining bcm2835_rng::get_random_bytes() in a new include/hw/misc/rng.h? (This can be later patch) > + > static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) > { > AspeedSCUState *s = ASPEED_SCU(opaque); > @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr > offset, unsigned size) > } > > switch (reg) { case RNG_CTRL: /* The RNG_DATA returns a different number regardless of * the state of the enabled bit in RNG_CTRL, * so the model follows this behaviour. */ break; With a comment: Reviewed-by: Philippe Mathieu-Daudé > +case RNG_DATA: > +s->regs[RNG_DATA] = aspeed_scu_get_random(); > +break; > case WAKEUP_EN: > qemu_log_mask(LOG_GUEST_ERROR, >"%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
On 23/05/2018 21:50, Patryk Olszewski wrote: > This patch fixes bug in serial that made it almost impossible for guest > to communicate with devices through host's serial. > > OPOST flag in c_oflag enables output processing letting other flags in > c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which > causes crlf to be sent in place of lf. This breaks binary transmissions. > Unsetting OPOST flag turns off any output processing which fixes the bug. > > Bug reports related: > https://bugs.launchpad.net/qemu/+bug/1772086 > https://bugs.launchpad.net/qemu/+bug/1407813 > https://bugs.launchpad.net/qemu/+bug/1715296 > also > https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html > > Signed-off-by: Patryk Olszewski> --- > chardev/char-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-serial.c b/chardev/char-serial.c > index feb52e5..ae548d2 100644 > --- a/chardev/char-serial.c > +++ b/chardev/char-serial.c > @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, > > tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP > | INLCR | IGNCR | ICRNL | IXON); > -tty.c_oflag |= OPOST; > +tty.c_oflag &= ~OPOST; > tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN | ISIG); > tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); > switch (data_bits) { > The same code was also copied in chardev/char-stdio.c, I fixed up that one too. Paolo
Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
On 24/05/2018 07:36, Thomas Huth wrote: > On 23.05.2018 21:50, Patryk Olszewski wrote: >> This patch fixes bug in serial that made it almost impossible for guest >> to communicate with devices through host's serial. >> >> OPOST flag in c_oflag enables output processing letting other flags in >> c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which >> causes crlf to be sent in place of lf. This breaks binary transmissions. >> Unsetting OPOST flag turns off any output processing which fixes the bug. >> >> Bug reports related: >> https://bugs.launchpad.net/qemu/+bug/1772086 >> https://bugs.launchpad.net/qemu/+bug/1407813 >> https://bugs.launchpad.net/qemu/+bug/1715296 >> also >> https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html >> >> Signed-off-by: Patryk Olszewski>> --- >> chardev/char-serial.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/chardev/char-serial.c b/chardev/char-serial.c >> index feb52e5..ae548d2 100644 >> --- a/chardev/char-serial.c >> +++ b/chardev/char-serial.c >> @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, >> >> tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP >> | INLCR | IGNCR | ICRNL | IXON); >> -tty.c_oflag |= OPOST; >> +tty.c_oflag &= ~OPOST; >> tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN | ISIG); >> tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); >> switch (data_bits) { >> > > I think this bug has good chances to win the price "Oldest bug of the > year". If we'd wait one more months, it has its 15th anniversary: The > code has originally been added in 2003, see commit 0824d6fc674084519c85, > it has just been moved around to other files afterwards. Thus this is > almost as old as QEMU itself! It was also already fixed once in commit 64b7b7334b92c1fdc1bb7d3d1afc342656c59539 Author: aurel32 Date: Mon May 5 10:05:31 2008 + Put Pseudo-TTY in rawmode for char devices (Daniel P. Berrange) Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH 4/4] spapr: introduce a new IRQ backend using fixed IRQ number ranges
On 05/28/2018 05:18 PM, Greg Kurz wrote: > On Fri, 18 May 2018 18:44:05 +0200 > Cédric Le Goaterwrote: > >> The proposed layout of the IRQ number space is organized as follow : >> >>RANGES DEVICES >> >>0x - 0x0FFFReserved for future use (IPI = 2) >>0x1000 - 0x10001 EPOW >>0x1001 - 0x10011 HOTPLUG >>0x1002 - 0x10FFunused >>0x1100 - 0x11FF256 VIO devices(1 IRQ each) >>0x1200 - 0x128332 PCI LSI devices (4 IRQs each) >>0x1284 - 0x13FFunused >>0x1400 - 0x17FFPCI MSI device 1 (1024 IRQs each) >>0x1800 - 0x1BFFPCI MSI device 1 > > device 2 and... > >>0x1c00 - 0x1FFFPCI MSI device 2 > > device 3 ? ah yes :) > >> >>0x2000 not allocated. Need to increase NR_IRQS >> > > What's NR_IRQS ? The total number of IRQs in the backend. > What's the goal to have several 1k ranges ? to support multiple PHBs with different allocatable MSI ranges, which is not the case today. > So that each one may be affected to a single device ? yes. >> The MSI range is a bit more complex to handle as the IRQS are dynamically >> allocated by the guest OS. In consequence, we use a bitmap allocator >> under the machine for these. >> >> The XICS IRQ number space is increased to 4K, which gives three MSI >> ranges of 1K for the PHBs. The XICS source IRQ numbers still have the >> 4K offset. >> > > Why ? It's the legacy IRQ offset value for the sPAPR sources (2 being reserved for XICS IPIs) and because XIVE will use the range for IPIs : 0x - 0x0FFFReserved for future use (IPI = 2) So not changing the value serve our purpose. >> Signed-off-by: Cédric Le Goater >> --- >> include/hw/ppc/spapr.h | 2 + >> include/hw/ppc/spapr_irq.h | 12 +++ >> hw/ppc/spapr.c | 22 + >> hw/ppc/spapr_irq.c | 220 >> - >> 4 files changed, 255 insertions(+), 1 deletion(-) >> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> index 4eb212b16a51..fcc1b1c1451d 100644 >> --- a/include/hw/ppc/spapr.h >> +++ b/include/hw/ppc/spapr.h >> @@ -165,6 +165,8 @@ struct sPAPRMachineState { >> char *kvm_type; >> MemoryHotplugState hotplug_memory; >> >> +int32_t nr_irqs; >> +unsigned long *irq_map; >> const char *icp_type; >> >> bool cmd_line_caps[SPAPR_CAP_NUM]; >> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h >> index caf4c33d4cec..d1af4c4d11ba 100644 >> --- a/include/hw/ppc/spapr_irq.h >> +++ b/include/hw/ppc/spapr_irq.h >> @@ -22,8 +22,16 @@ >> #define SPAPR_IRQ_PCI_MSI0x1400 /* 1K IRQs per device covered by >>* a bitmap allocator */ >> >> +typedef struct sPAPRPIrqRange { >> +const char *name; >> +uint32_t offset; >> +uint32_t width; >> +uint32_t max_index; >> +} sPAPRPIrqRange; >> + >> typedef struct sPAPRIrq { >> uint32_tnr_irqs; >> +const sPAPRPIrqRange *ranges; >> >> void (*init)(sPAPRMachineState *spapr, Error **errp); >> int (*alloc)(sPAPRMachineState *spapr, uint32_t range, uint32_t index, >> @@ -35,6 +43,10 @@ typedef struct sPAPRIrq { >> } sPAPRIrq; >> >> extern sPAPRIrq spapr_irq_default; >> +extern sPAPRIrq spapr_irq_2_12; >> + >> +const sPAPRPIrqRange *spapr_irq_get_range(sPAPRMachineState *spapr, >> + uint32_t offset); >> >> int spapr_irq_alloc(sPAPRMachineState *spapr, uint32_t range, uint32_t >> index, >> Error **errp); >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 09f095d73eae..f2ebd6f20414 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1848,6 +1848,24 @@ static const VMStateDescription >> vmstate_spapr_patb_entry = { >> }, >> }; >> >> +static bool spapr_irq_map_needed(void *opaque) >> +{ >> +sPAPRMachineState *spapr = opaque; >> + >> +return spapr->irq_map && !bitmap_empty(spapr->irq_map, spapr->nr_irqs); >> +} >> + >> +static const VMStateDescription vmstate_spapr_irq_map = { >> +.name = "spapr_irq_map", >> +.version_id = 1, >> +.minimum_version_id = 1, >> +.needed = spapr_irq_map_needed, >> +.fields = (VMStateField[]) { >> +VMSTATE_BITMAP(irq_map, sPAPRMachineState, 0, nr_irqs), >> +VMSTATE_END_OF_LIST() >> +}, >> +}; >> + >> static const VMStateDescription vmstate_spapr = { >> .name = "spapr", >> .version_id = 3, >> @@ -1875,6 +1893,7 @@ static const VMStateDescription vmstate_spapr = { >> _spapr_cap_cfpc, >> _spapr_cap_sbbc, >> _spapr_cap_ibs, >> +_spapr_irq_map, >> NULL >> } >> }; >> @@ -3916,7 +3935,10 @@ static void >> spapr_machine_2_12_instance_options(MachineState *machine) >> >> static void spapr_machine_2_12_class_options(MachineClass *mc) >> { >> +sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); >> +
Re: [Qemu-devel] [PATCH v4 0/4] qdev: remove DeviceClass::init/exit()
On 05/28/2018 12:39 PM, Paolo Bonzini wrote: > On 28/05/2018 16:45, Markus Armbruster wrote: >> This lovely series got stuck after v3, so I took the liberty to respin >> it. >> >> v4: >> * PATCH 1+2 unchanged >> * PATCH 3+4 reshuffled a bit, missing documentation updates supplied > > At long last! Queued,t hanks. Thanks both of you! signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 0/4] qdev: remove DeviceClass::init/exit()
On 28/05/2018 16:45, Markus Armbruster wrote: > This lovely series got stuck after v3, so I took the liberty to respin > it. > > v4: > * PATCH 1+2 unchanged > * PATCH 3+4 reshuffled a bit, missing documentation updates supplied At long last! Queued,t hanks. Paolo
Re: [Qemu-devel] [PATCH v2] aspeed_scu: Implement RNG register
On 05/28/2018 05:22 PM, Joel Stanley wrote: > The ASPEED SoCs contain a single register that returns random data when > read. This models that register so that guests can use it. > > The random number data register has a corresponding control register, > data returns a different number regardless of the state of the enabled > bit, so the model follows this behaviour. > > Signed-off-by: Joel StanleyReviewed-by: Cédric Le Goater > --- > v2: > - Remove call to qcrypto_random_init as this is done in main() > --- > hw/misc/aspeed_scu.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 5e6d5744eeca..29e58527793b 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -16,6 +16,7 @@ > #include "qapi/visitor.h" > #include "qemu/bitops.h" > #include "qemu/log.h" > +#include "crypto/random.h" > #include "trace.h" > > #define TO_REG(offset) ((offset) >> 2) > @@ -154,6 +155,18 @@ static const uint32_t > ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { > [BMC_DEV_ID] = 0x2402U > }; > > +static uint32_t aspeed_scu_get_random(void) > +{ > +Error *err = NULL; > +uint32_t num; > + > +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { > +error_report_err(err); > +} > + > +return num; > +} > + > static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) > { > AspeedSCUState *s = ASPEED_SCU(opaque); > @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr > offset, unsigned size) > } > > switch (reg) { > +case RNG_DATA: > +s->regs[RNG_DATA] = aspeed_scu_get_random(); > +break; > case WAKEUP_EN: > qemu_log_mask(LOG_GUEST_ERROR, >"%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", >
Re: [Qemu-devel] [RFC PATCH] mmio-exec: Make device return MemoryRegion rather than host pointer
On 05/28/2018 07:22 AM, Cédric Le Goater wrote: > On 04/26/2018 05:09 PM, Peter Maydell wrote: >> Our current interface for allowing a device to support execution from >> MMIO regions has the device return a pointer into host memory >> containing the contents to be used for execution. Unfortunately the >> obvious implementation this encourages (and which our only in-tree >> user uses) is to have a single buffer which is reused for each new >> request_ptr call. This doesn't work, because RCU means that removal >> of the old RAMBlock based on the host pointer may be deferred until a >> point after a new RAMBlock with the same pointer has been added to >> the list. The RAMBlock APIs assume that the host-pointer to ram_addr >> mapping is unique, and mmio-exec is breaking that assumption. > > yes. I have run into the same problem while implementing mmio-exec > for the aspeed fmc controllers. > > The FW (U-boot) in some configuration does a CRC calculation which > jumps from one code section to another. This trashes the MMIO cache > a very large number of times and the RAM block list can contain up > to 40 occurrences of the same cache ... it is nearly impossible not > to hit the bug you describe. For the records, there are more than 31000 request/invalidate calls. > This patch fixes the issue There is still an issue (or a limitation) with the patch. It is not possible to reload the cache when a load is performed on the MMIO region from which QEMU is exec'ing. This breaks the TCG flow it seems. > but maybe an another approach could be to introduce a cache allocator > which > would make sure that host-pointers are unique ? but freeing these cache lines is a problem. Same as of today ... Hmm, really a complex problem. Thanks, C. >> Instead, change the API so that the device is responsible for >> allocating a new RAM MemoryRegion and populating it. The >> MemoryRegion will be g_free()d when the region is eventually >> invalidated or otherwise unneeded. >> >> HACKS: >> * Note that in order for the refcounting to work correctly without >>having to manufacture a device purely to be the owner of the >>MemoryRegion, we must pass OBJECT(newmr) as the owner (so the MR is >>its own owner!) and NULL as the name (or the property created on the >>owner causes the refcount to go negative during finalization)... >> >> QUESTIONS: >> * do we really need to postpone things from >>memory_region_invalidate_mmio_ptr() to >>memory_region_do_invalidate_mmio_ptr(), given that the deletion of >>the memory region and ramblock is RCU-deferred anyway? >> * should we add the subregion at elevated prio so it definitely hits >>first? I've left it the way the existing code does for the >>moment... >> * is there any way to avoid the weird self-owning MemoryRegion >>and corresponding need to pass a NULL name pointer? >> >> NOTES: >> * This change means that hw/misc/mmio_interface.c is no longer >>used; if this gets beyond RFC stage then another patch to >>delete it can follow. >> * The "request_mmio_exec mustn't fail after it has called >>memory_region_mmio_ptr_alloc()" requirement is a bit ugly, but >>could probably be removed. >> >> Signed-off-by: Peter Maydell>> --- >> This is decidedly RFC, but it is sufficient to get the xilinx-spips >> mmio-execution test case to run without crashing. Mostly looking for >> feedback on whether this is the right general direction or a load >> of rubbish :-) >> >> include/exec/memory.h | 44 >> hw/ssi/xilinx_spips.c | 22 >> memory.c | 78 --- >> 3 files changed, 105 insertions(+), 39 deletions(-) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 31eae0a640..e55e06a166 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -139,14 +139,32 @@ struct MemoryRegionOps { >> unsigned size, >> MemTxAttrs attrs); >> /* Instruction execution pre-callback: >> - * @addr is the address of the access relative to the @mr. >> - * @size is the size of the area returned by the callback. >> - * @offset is the location of the pointer inside @mr. >> + * @opaque is the opaque pointer for the MemoryRegion. >> + * @alloc_token is a token to pass to memory_region_mmio_ptr_alloc() >> + * @offset contains the address of the access relative to the @mr. >> * >> - * Returns a pointer to a location which contains guest code. >> + * The request_mmio_exec callback must: >> + * - adjust the offset downwards as desired and determine the size >> + *of the region it wants to provide cached guest code for. >> + *This must start on a guest page boundary and be a multiple >> + *of a guest page in size. >> + * - call
[Qemu-devel] [PATCH v2] aspeed_scu: Implement RNG register
The ASPEED SoCs contain a single register that returns random data when read. This models that register so that guests can use it. The random number data register has a corresponding control register, data returns a different number regardless of the state of the enabled bit, so the model follows this behaviour. Signed-off-by: Joel Stanley--- v2: - Remove call to qcrypto_random_init as this is done in main() --- hw/misc/aspeed_scu.c | 16 1 file changed, 16 insertions(+) diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 5e6d5744eeca..29e58527793b 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -16,6 +16,7 @@ #include "qapi/visitor.h" #include "qemu/bitops.h" #include "qemu/log.h" +#include "crypto/random.h" #include "trace.h" #define TO_REG(offset) ((offset) >> 2) @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { [BMC_DEV_ID] = 0x2402U }; +static uint32_t aspeed_scu_get_random(void) +{ +Error *err = NULL; +uint32_t num; + +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { +error_report_err(err); +} + +return num; +} + static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) } switch (reg) { +case RNG_DATA: +s->regs[RNG_DATA] = aspeed_scu_get_random(); +break; case WAKEUP_EN: qemu_log_mask(LOG_GUEST_ERROR, "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", -- 2.17.0
Re: [Qemu-devel] [PATCH v7 4/4] monitor: add lock to protect mon_fdsets
Peter Xuwrites: > On Thu, May 24, 2018 at 11:03:55AM +0200, Markus Armbruster wrote: >> Peter Xu writes: >> >> > Similar to previous patch, but introduce a new global big lock for >> > mon_fdsets. Take it where needed. >> >> The previous patch is "monitor: more comments on lock-free >> fleids/funcs". Sure you mean that one? > > No... I'll remove that sentence directly: > > "Introduce a new global big lock for mon_fdsets. Take it where needed." Works for me. >> > The monitor_fdset_get_fd() handling is a bit tricky: now we need to call >> > qemu_mutex_unlock() which might pollute errno, so we need to make sure >> > the correct errno be passed up to the callers. To make things simpler, >> > we let monitor_fdset_get_fd() return the -errno directly when error >> > happens, then in qemu_open() we translate it back into errno. >> >> Suggest s/translate/move/. > > Okay. > >> > >> > Signed-off-by: Peter Xu >> > --- >> > monitor.c| 70 +--- >> > util/osdep.c | 3 ++- >> > 2 files changed, 58 insertions(+), 15 deletions(-) >> > >> > diff --git a/monitor.c b/monitor.c >> > index f01589f945..6266ff65c4 100644 >> > --- a/monitor.c >> > +++ b/monitor.c >> > @@ -271,6 +271,9 @@ typedef struct QMPRequest QMPRequest; >> > /* Protects mon_list, monitor_event_state. */ >> >> Not this patch's problem: there is no monitor_event_state. Screwed up >> in commit d622cb5879c. I guess it's monitor_qapi_event_state. > > I'll append another patch to do that, and move these fields together. > >> >> > static QemuMutex monitor_lock; >> > >> > +/* Protects mon_fdsets */ >> > +static QemuMutex mon_fdsets_lock; >> > + >> > static QTAILQ_HEAD(mon_list, Monitor) mon_list; >> > static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; >> > static int mon_refcount; >> >> Suggest to move mon_fdsets next to the lock protecting it. > > Will do. > >> >> > @@ -287,6 +290,16 @@ static QEMUClockType event_clock_type = >> > QEMU_CLOCK_REALTIME; >> > static void monitor_command_cb(void *opaque, const char *cmdline, >> > void *readline_opaque); >> > >> > +/* >> > + * This lock can be used very early, even during param parsing. >> >> Do you mean CLI parsing? > > Yes, will s/param/CLI/. > >> >> > + * Meanwhile it can also be used even at the end of main. Let's keep >> > + * it initialized for the whole lifecycle of QEMU. >> > + */ >> >> Awkward question, since our main() is such a tangled mess, but here goes >> anyway... The existing place to initialize monitor.c's globals is >> monitor_init_globals(). But that one runs too late, I guess: >> parse_add_fd() runs earlier, and calls monitor_fdset_add_fd(). Unclean >> even without this lock; no module should be used before its >> initialization function runs. Sure we can't run monitor_init_globals() >> sufficiently early? > > Please see the comment for monitor_init_globals(): > > /* > * Note: qtest_enabled() (which is used in monitor_qapi_event_init()) > * depends on configure_accelerator() above. > */ > monitor_init_globals(); > > So I guess it won't work to directly move it earlier. The init > dependency of QEMU is really complicated. I'll be fine now that we > mark totally independent init functions (like this one, which is a > mutex init only) as constructor, then we can save some time on > ordering issue. Let me rephrase. There's a preexisting issue: main() calls monitor.c functions before calling its initialization function monitor_init_globals(). This needs to be cleaned up. Would you be willing to do it? Possible solutions: * Perhaps we can move monitor_init_globals() up and/or the code calling into monitor.c early down sufficiently. * Calculate event_clock_type on each use instead of ahead of time. It's qtest_enabled ? QEMU_CLOCK_VIRTUAL : QEMU_CLOCK_REALTIME, and neither of its users needs to be fast. Then move monitor_init_globals before the code calling into monitor.c. I'm not opposed to use of constructors for self-contained initialization (no calls to other modules). But I don't like initialization spread over multiple functions. >> > +static void __attribute__((constructor)) mon_fdsets_lock_init(void) >> > +{ >> > +qemu_mutex_init(_fdsets_lock); >> > +} >> > + >> > /** >> > * Is @mon a QMP monitor? >> > */ >> > @@ -2316,9 +2329,11 @@ static void monitor_fdsets_cleanup(void) >> > MonFdset *mon_fdset; >> > MonFdset *mon_fdset_next; >> > >> > +qemu_mutex_lock(_fdsets_lock); >> > QLIST_FOREACH_SAFE(mon_fdset, _fdsets, next, mon_fdset_next) { >> > monitor_fdset_cleanup(mon_fdset); >> > } >> > +qemu_mutex_unlock(_fdsets_lock); >> > } >> > >> > AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool >> > has_opaque, >> > @@ -2353,6 +2368,7 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, >> > int64_t fd, Error **errp)
Re: [Qemu-devel] [PATCH 4/4] spapr: introduce a new IRQ backend using fixed IRQ number ranges
On Fri, 18 May 2018 18:44:05 +0200 Cédric Le Goaterwrote: > The proposed layout of the IRQ number space is organized as follow : > >RANGES DEVICES > >0x - 0x0FFFReserved for future use (IPI = 2) >0x1000 - 0x10001 EPOW >0x1001 - 0x10011 HOTPLUG >0x1002 - 0x10FFunused >0x1100 - 0x11FF256 VIO devices(1 IRQ each) >0x1200 - 0x128332 PCI LSI devices (4 IRQs each) >0x1284 - 0x13FFunused >0x1400 - 0x17FFPCI MSI device 1 (1024 IRQs each) >0x1800 - 0x1BFFPCI MSI device 1 device 2 and... >0x1c00 - 0x1FFFPCI MSI device 2 device 3 ? > >0x2000 not allocated. Need to increase NR_IRQS > What's NR_IRQS ? What's the goal to have several 1k ranges ? So that each one may be affected to a single device ? > The MSI range is a bit more complex to handle as the IRQS are dynamically > allocated by the guest OS. In consequence, we use a bitmap allocator > under the machine for these. > > The XICS IRQ number space is increased to 4K, which gives three MSI > ranges of 1K for the PHBs. The XICS source IRQ numbers still have the > 4K offset. > Why ? > Signed-off-by: Cédric Le Goater > --- > include/hw/ppc/spapr.h | 2 + > include/hw/ppc/spapr_irq.h | 12 +++ > hw/ppc/spapr.c | 22 + > hw/ppc/spapr_irq.c | 220 > - > 4 files changed, 255 insertions(+), 1 deletion(-) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 4eb212b16a51..fcc1b1c1451d 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -165,6 +165,8 @@ struct sPAPRMachineState { > char *kvm_type; > MemoryHotplugState hotplug_memory; > > +int32_t nr_irqs; > +unsigned long *irq_map; > const char *icp_type; > > bool cmd_line_caps[SPAPR_CAP_NUM]; > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index caf4c33d4cec..d1af4c4d11ba 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -22,8 +22,16 @@ > #define SPAPR_IRQ_PCI_MSI0x1400 /* 1K IRQs per device covered by >* a bitmap allocator */ > > +typedef struct sPAPRPIrqRange { > +const char *name; > +uint32_t offset; > +uint32_t width; > +uint32_t max_index; > +} sPAPRPIrqRange; > + > typedef struct sPAPRIrq { > uint32_tnr_irqs; > +const sPAPRPIrqRange *ranges; > > void (*init)(sPAPRMachineState *spapr, Error **errp); > int (*alloc)(sPAPRMachineState *spapr, uint32_t range, uint32_t index, > @@ -35,6 +43,10 @@ typedef struct sPAPRIrq { > } sPAPRIrq; > > extern sPAPRIrq spapr_irq_default; > +extern sPAPRIrq spapr_irq_2_12; > + > +const sPAPRPIrqRange *spapr_irq_get_range(sPAPRMachineState *spapr, > + uint32_t offset); > > int spapr_irq_alloc(sPAPRMachineState *spapr, uint32_t range, uint32_t index, > Error **errp); > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 09f095d73eae..f2ebd6f20414 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1848,6 +1848,24 @@ static const VMStateDescription > vmstate_spapr_patb_entry = { > }, > }; > > +static bool spapr_irq_map_needed(void *opaque) > +{ > +sPAPRMachineState *spapr = opaque; > + > +return spapr->irq_map && !bitmap_empty(spapr->irq_map, spapr->nr_irqs); > +} > + > +static const VMStateDescription vmstate_spapr_irq_map = { > +.name = "spapr_irq_map", > +.version_id = 1, > +.minimum_version_id = 1, > +.needed = spapr_irq_map_needed, > +.fields = (VMStateField[]) { > +VMSTATE_BITMAP(irq_map, sPAPRMachineState, 0, nr_irqs), > +VMSTATE_END_OF_LIST() > +}, > +}; > + > static const VMStateDescription vmstate_spapr = { > .name = "spapr", > .version_id = 3, > @@ -1875,6 +1893,7 @@ static const VMStateDescription vmstate_spapr = { > _spapr_cap_cfpc, > _spapr_cap_sbbc, > _spapr_cap_ibs, > +_spapr_irq_map, > NULL > } > }; > @@ -3916,7 +3935,10 @@ static void > spapr_machine_2_12_instance_options(MachineState *machine) > > static void spapr_machine_2_12_class_options(MachineClass *mc) > { > +sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); > + > spapr_machine_2_13_class_options(mc); > +smc->irq = _irq_2_12; > SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12); > } > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index ff6cb1aafd25..bfffb1467336 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -192,7 +192,7 @@ static qemu_irq spapr_qirq_2_12(sPAPRMachineState *spapr, > int irq) > return NULL; > } > > -sPAPRIrq spapr_irq_default = { > +sPAPRIrq spapr_irq_2_12 = { > .nr_irqs = XICS_IRQS_SPAPR, > .init= spapr_irq_init_2_12, > .alloc =
[Qemu-devel] [PATCH] qcow2: Fix Coverity warning when calculating the refcount cache size
MIN_REFCOUNT_CACHE_SIZE is 4 and the cluster size is guaranteed to be at most 2MB, so the minimum refcount cache size (in bytes) is always going to fit in a 32-bit integer. Coverity doesn't know that, and since we're storing the result in a uint64_t (*refcount_cache_size) it thinks that we need the 64 bits and that we probably want to do a 64-bit multiplication to prevent the result from being truncated. This is a false positive in this case, but it's a fair warning. We could do a 64-bit multiplication to get rid of it, but since we know that a 32-bit variable is enough to store this value let's simply reuse min_refcount_cache, make it a normal int and stop doing casts. Signed-off-by: Alberto GarciaReported-by: Peter Maydell --- block/qcow2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 6d532470a8..a007dc4246 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -768,6 +768,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts, BDRVQcow2State *s = bs->opaque; uint64_t combined_cache_size; bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set; +int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE); l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE); @@ -804,8 +805,6 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts, } else { uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE; uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8); -uint64_t min_refcount_cache = -(uint64_t) MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; /* Assign as much memory as possible to the L2 cache, and * use the remainder for the refcount cache */ @@ -825,7 +824,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts, * s->cluster_size); } if (!refcount_cache_size_set) { -*refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; +*refcount_cache_size = min_refcount_cache; } } -- 2.11.0
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
On 05/28/2018 04:29 PM, Joel Stanley wrote: > On 28 May 2018 at 23:33, Joel Stanleywrote: >> On 28 May 2018 at 23:17, Cédric Le Goater wrote: >>> Hello Joel, >>> >>> On 05/28/2018 02:46 PM, Joel Stanley wrote: The ASPEED SoCs contain a single register that returns random data when read. This models that register so that guests can use it. Signed-off-by: Joel Stanley --- hw/misc/aspeed_scu.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 5e6d5744eeca..8fa0cecf0fa1 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -16,6 +16,7 @@ #include "qapi/visitor.h" #include "qemu/bitops.h" #include "qemu/log.h" +#include "crypto/random.h" #include "trace.h" #define TO_REG(offset) ((offset) >> 2) @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { [BMC_DEV_ID] = 0x2402U }; +static uint32_t aspeed_scu_get_random(void) +{ +Error *err = NULL; +uint32_t num; + +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { +error_report_err(err); +} + +return num; +} + static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) } switch (reg) { +case RNG_DATA: +return aspeed_scu_get_random(); >>> >>> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not. >> >> The RNG is enabled by default, and I didn't find any software that >> disables it, but it can't hurt to have that check. > > I did some testing on hardware, and the rng still outputs a different > number each time I ask for one regardless of the state of the enabled > bit. > > How should we model that? > I confirm that the HW doesn't really care about the enabled bit. Let's ignore it then ? C.
[Qemu-devel] [PATCH v4 2/4] hw/i2c: Use DeviceClass::realize instead of I2CSlaveClass::init
From: Philippe Mathieu-DaudéI2CSlaveClass::init is no more used, remove it. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20180419212727.26095-3-f4...@amsat.org> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- hw/audio/wm8750.c | 8 +++- hw/display/ssd0303.c| 9 - hw/gpio/max7310.c | 9 - hw/i2c/core.c | 13 - hw/input/lm832x.c | 9 - hw/misc/tmp105.c| 7 +++ hw/misc/tmp421.c| 8 +++- hw/nvram/eeprom_at24c.c | 24 +++- hw/timer/twl92230.c | 11 --- include/hw/i2c/i2c.h| 3 --- 10 files changed, 36 insertions(+), 65 deletions(-) diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c index 416a78e869..f4aa838f62 100644 --- a/hw/audio/wm8750.c +++ b/hw/audio/wm8750.c @@ -617,14 +617,12 @@ static const VMStateDescription vmstate_wm8750 = { } }; -static int wm8750_init(I2CSlave *i2c) +static void wm8750_realize(DeviceState *dev, Error **errp) { -WM8750State *s = WM8750(i2c); +WM8750State *s = WM8750(dev); AUD_register_card(CODEC, >card); wm8750_reset(I2C_SLAVE(s)); - -return 0; } #if 0 @@ -707,7 +705,7 @@ static void wm8750_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); -sc->init = wm8750_init; +dc->realize = wm8750_realize; sc->event = wm8750_event; sc->recv = wm8750_rx; sc->send = wm8750_tx; diff --git a/hw/display/ssd0303.c b/hw/display/ssd0303.c index 68a80b9d64..eb90ba26be 100644 --- a/hw/display/ssd0303.c +++ b/hw/display/ssd0303.c @@ -297,13 +297,12 @@ static const GraphicHwOps ssd0303_ops = { .gfx_update = ssd0303_update_display, }; -static int ssd0303_init(I2CSlave *i2c) +static void ssd0303_realize(DeviceState *dev, Error **errp) { -ssd0303_state *s = SSD0303(i2c); +ssd0303_state *s = SSD0303(dev); -s->con = graphic_console_init(DEVICE(i2c), 0, _ops, s); +s->con = graphic_console_init(dev, 0, _ops, s); qemu_console_resize(s->con, 96 * MAGNIFY, 16 * MAGNIFY); -return 0; } static void ssd0303_class_init(ObjectClass *klass, void *data) @@ -311,7 +310,7 @@ static void ssd0303_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); -k->init = ssd0303_init; +dc->realize = ssd0303_realize; k->event = ssd0303_event; k->recv = ssd0303_recv; k->send = ssd0303_send; diff --git a/hw/gpio/max7310.c b/hw/gpio/max7310.c index 4c203ef5c6..a560e3afd2 100644 --- a/hw/gpio/max7310.c +++ b/hw/gpio/max7310.c @@ -182,14 +182,13 @@ static void max7310_gpio_set(void *opaque, int line, int level) /* MAX7310 is SMBus-compatible (can be used with only SMBus protocols), * but also accepts sequences that are not SMBus so return an I2C device. */ -static int max7310_init(I2CSlave *i2c) +static void max7310_realize(DeviceState *dev, Error **errp) { -MAX7310State *s = MAX7310(i2c); +I2CSlave *i2c = I2C_SLAVE(dev); +MAX7310State *s = MAX7310(dev); qdev_init_gpio_in(>qdev, max7310_gpio_set, 8); qdev_init_gpio_out(>qdev, s->handler, 8); - -return 0; } static void max7310_class_init(ObjectClass *klass, void *data) @@ -197,7 +196,7 @@ static void max7310_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); -k->init = max7310_init; +dc->realize = max7310_realize; k->event = max7310_event; k->recv = max7310_rx; k->send = max7310_tx; diff --git a/hw/i2c/core.c b/hw/i2c/core.c index cfccefca3d..ab72d5bf2b 100644 --- a/hw/i2c/core.c +++ b/hw/i2c/core.c @@ -258,18 +258,6 @@ const VMStateDescription vmstate_i2c_slave = { } }; -static int i2c_slave_qdev_init(DeviceState *dev) -{ -I2CSlave *s = I2C_SLAVE(dev); -I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(s); - -if (sc->init) { -return sc->init(s); -} - -return 0; -} - DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) { DeviceState *dev; @@ -283,7 +271,6 @@ DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) static void i2c_slave_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); -k->init = i2c_slave_qdev_init; set_bit(DEVICE_CATEGORY_MISC, k->categories); k->bus_type = TYPE_I2C_BUS; k->props = i2c_props; diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c index d39953126b..74da30d9ca 100644 --- a/hw/input/lm832x.c +++ b/hw/input/lm832x.c @@ -464,20 +464,19 @@ static const VMStateDescription vmstate_lm_kbd = { }; -static int lm8323_init(I2CSlave *i2c) +static void lm8323_realize(DeviceState *dev, Error **errp) { -LM823KbdState *s = LM8323(i2c); +LM823KbdState *s =
[Qemu-devel] [PATCH v4 4/4] qdev: Remove DeviceClass::init() and ::exit()
From: Philippe Mathieu-DaudéSince no devices use it, we can safely remove it. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20180419212727.26095-5-f4...@amsat.org> Reviewed-by: Markus Armbruster [Removal of DeviceClass::init() moved from previous patch, missing documentation updates supplied] Signed-off-by: Markus Armbruster --- hw/core/qdev.c | 28 include/hw/qdev-core.h | 20 2 files changed, 4 insertions(+), 44 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index f6f92473b8..ffec461791 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -208,32 +208,6 @@ void device_listener_unregister(DeviceListener *listener) QTAILQ_REMOVE(_listeners, listener, link); } -static void device_realize(DeviceState *dev, Error **errp) -{ -DeviceClass *dc = DEVICE_GET_CLASS(dev); - -if (dc->init) { -int rc = dc->init(dev); -if (rc < 0) { -error_setg(errp, "Device initialization failed."); -return; -} -} -} - -static void device_unrealize(DeviceState *dev, Error **errp) -{ -DeviceClass *dc = DEVICE_GET_CLASS(dev); - -if (dc->exit) { -int rc = dc->exit(dev); -if (rc < 0) { -error_setg(errp, "Device exit failed."); -return; -} -} -} - void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, int required_for_version) { @@ -1065,8 +1039,6 @@ static void device_class_init(ObjectClass *class, void *data) DeviceClass *dc = DEVICE_CLASS(class); class->unparent = device_unparent; -dc->realize = device_realize; -dc->unrealize = device_unrealize; /* by default all devices were considered as hotpluggable, * so with intent to check it in generic qdev_unplug() / diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 9453588160..f1fd0f8736 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -29,8 +29,6 @@ typedef enum DeviceCategory { DEVICE_CATEGORY_MAX } DeviceCategory; -typedef int (*qdev_initfn)(DeviceState *dev); -typedef int (*qdev_event)(DeviceState *dev); typedef void (*DeviceRealize)(DeviceState *dev, Error **errp); typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp); typedef void (*DeviceReset)(DeviceState *dev); @@ -43,13 +41,9 @@ struct VMStateDescription; * DeviceClass: * @props: Properties accessing state fields. * @realize: Callback function invoked when the #DeviceState:realized - * property is changed to %true. The default invokes @init if not %NULL. + * property is changed to %true. * @unrealize: Callback function invoked when the #DeviceState:realized * property is changed to %false. - * @init: Callback function invoked when the #DeviceState::realized property - * is changed to %true. Deprecated, new types inheriting directly from - * TYPE_DEVICE should use @realize instead, new leaf types should consult - * their respective parent type. * @hotpluggable: indicates if #DeviceClass is hotpluggable, available * as readonly "hotpluggable" property of #DeviceState instance * @@ -73,19 +67,15 @@ struct VMStateDescription; * object_initialize() in their own #TypeInfo.instance_init and forward the * realization events appropriately. * - * The @init callback is considered private to a particular bus implementation - * (immediate abstract child types of TYPE_DEVICE). Derived leaf types set an - * "init" callback on their parent class instead. - * * Any type may override the @realize and/or @unrealize callbacks but needs * to call the parent type's implementation if keeping their functionality * is desired. Refer to QOM documentation for further discussion and examples. * * * - * If a type derived directly from TYPE_DEVICE implements @realize, it does - * not need to implement @init and therefore does not need to store and call - * #DeviceClass' default @realize callback. + * Since TYPE_DEVICE doesn't implement @realize and @unrealize, types + * derived directly from it need not call their parent's @realize and + * @unrealize. * For other types consult the documentation and implementation of the * respective parent types. * @@ -124,8 +114,6 @@ typedef struct DeviceClass { const struct VMStateDescription *vmsd; /* Private to qdev / bus. */ -qdev_initfn init; /* TODO remove, once users are converted to realize */ -qdev_event exit; /* TODO remove, once users are converted to unrealize */ const char *bus_type; } DeviceClass; -- 2.13.6
[Qemu-devel] [PATCH v4 1/4] hw/i2c/smbus: Use DeviceClass::realize instead of SMBusDeviceClass::init
From: Philippe Mathieu-DaudéSMBusDeviceClass::init is no more used, remove it. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20180419212727.26095-2-f4...@amsat.org> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- hw/i2c/smbus.c | 9 - hw/i2c/smbus_eeprom.c | 5 ++--- include/hw/i2c/smbus.h | 1 - 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/hw/i2c/smbus.c b/hw/i2c/smbus.c index 2d1b79a689..587ce1ab7f 100644 --- a/hw/i2c/smbus.c +++ b/hw/i2c/smbus.c @@ -202,14 +202,6 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data) return 0; } -static int smbus_device_init(I2CSlave *i2c) -{ -SMBusDevice *dev = SMBUS_DEVICE(i2c); -SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - -return sc->init(dev); -} - /* Master device commands. */ int smbus_quick_command(I2CBus *bus, uint8_t addr, int read) { @@ -350,7 +342,6 @@ static void smbus_device_class_init(ObjectClass *klass, void *data) { I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); -sc->init = smbus_device_init; sc->event = smbus_i2c_event; sc->recv = smbus_i2c_recv; sc->send = smbus_i2c_send; diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index b13ec0fe7a..125c887d1f 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -97,12 +97,11 @@ static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t cmd, int n) return eeprom_receive_byte(dev); } -static int smbus_eeprom_initfn(SMBusDevice *dev) +static void smbus_eeprom_realize(DeviceState *dev, Error **errp) { SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev; eeprom->offset = 0; -return 0; } static Property smbus_eeprom_properties[] = { @@ -115,7 +114,7 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SMBusDeviceClass *sc = SMBUS_DEVICE_CLASS(klass); -sc->init = smbus_eeprom_initfn; +dc->realize = smbus_eeprom_realize; sc->quick_cmd = eeprom_quick_cmd; sc->send_byte = eeprom_send_byte; sc->receive_byte = eeprom_receive_byte; diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h index 544bbc1957..cfe3fa69f3 100644 --- a/include/hw/i2c/smbus.h +++ b/include/hw/i2c/smbus.h @@ -38,7 +38,6 @@ typedef struct SMBusDeviceClass { I2CSlaveClass parent_class; -int (*init)(SMBusDevice *dev); void (*quick_cmd)(SMBusDevice *dev, uint8_t read); void (*send_byte)(SMBusDevice *dev, uint8_t val); uint8_t (*receive_byte)(SMBusDevice *dev); -- 2.13.6
[Qemu-devel] [PATCH v4 3/4] qdev: Simplify the SysBusDeviceClass::init path
From: Philippe Mathieu-DaudéInstead of using SysBusDeviceClass::realize -> DeviceClass::realize -> DeviceClass::init -> sysbus_device_init -> SysBusDeviceClass::init Simplify the path by directly calling SysBusDeviceClass::init in SysBusDeviceClass::realize: SysBusDeviceClass::realize -> SysBusDeviceClass::init Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20180419212727.26095-4-f4...@amsat.org> Reviewed-by: Markus Armbruster [Removal of DeviceClass::init() moved into next patch, sysbus_realize() tweaked for clarity] Signed-off-by: Markus Armbruster --- hw/core/sysbus.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index 5d0887f499..ecfb0cfc0e 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/sysbus.h" #include "monitor/monitor.h" #include "exec/address-spaces.h" @@ -200,15 +201,18 @@ void sysbus_init_ioports(SysBusDevice *dev, uint32_t ioport, uint32_t size) } } -static int sysbus_device_init(DeviceState *dev) +/* TODO remove once all sysbus devices have been converted to realize */ +static void sysbus_realize(DeviceState *dev, Error **errp) { SysBusDevice *sd = SYS_BUS_DEVICE(dev); SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd); if (!sbc->init) { -return 0; +return; +} +if (sbc->init(sd) < 0) { +error_setg(errp, "Device initialization failed"); } -return sbc->init(sd); } DeviceState *sysbus_create_varargs(const char *name, @@ -324,7 +328,7 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev) static void sysbus_device_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); -k->init = sysbus_device_init; +k->realize = sysbus_realize; k->bus_type = TYPE_SYSTEM_BUS; /* * device_add plugs devices into a suitable bus. For "real" buses, -- 2.13.6
[Qemu-devel] [PATCH v4 0/4] qdev: remove DeviceClass::init/exit()
This lovely series got stuck after v3, so I took the liberty to respin it. v4: * PATCH 1+2 unchanged * PATCH 3+4 reshuffled a bit, missing documentation updates supplied Philippe's cover letter: Since v2: - rebased for 2.13 (Markus) - dropped 2 patches already merged (Gerd) - start sentences with a capital letter and end with a full stop (Peter) since v1: - fix format string on 32-bit host (patchew) - do not add smbus_eeprom_reset() (Eduardo) - directly use DeviceClass::realize (Eduardo) - squashed 2 patches (Eduardo) Hi, This series finalize the qdev QOMification. We first convert the I2CSlave/SMBusDevice, then the usb-ccid and virtio-ccw, and finally the SysBusDevice. At the end we removed *TWO* TODO :) /* TODO remove, once users are converted to realize */ /* TODO remove, once users are converted to unrealize */ Philippe Mathieu-Daudé (4): hw/i2c/smbus: Use DeviceClass::realize instead of SMBusDeviceClass::init hw/i2c: Use DeviceClass::realize instead of I2CSlaveClass::init qdev: Simplify the SysBusDeviceClass::init path qdev: Remove DeviceClass::init() and ::exit() hw/audio/wm8750.c | 8 +++- hw/core/qdev.c | 28 hw/core/sysbus.c| 12 hw/display/ssd0303.c| 9 - hw/gpio/max7310.c | 9 - hw/i2c/core.c | 13 - hw/i2c/smbus.c | 9 - hw/i2c/smbus_eeprom.c | 5 ++--- hw/input/lm832x.c | 9 - hw/misc/tmp105.c| 7 +++ hw/misc/tmp421.c| 8 +++- hw/nvram/eeprom_at24c.c | 24 +++- hw/timer/twl92230.c | 11 --- include/hw/i2c/i2c.h| 3 --- include/hw/i2c/smbus.h | 1 - include/hw/qdev-core.h | 20 16 files changed, 50 insertions(+), 126 deletions(-) -- 2.13.6
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
On 28 May 2018 at 23:33, Joel Stanleywrote: > On 28 May 2018 at 23:17, Cédric Le Goater wrote: >> Hello Joel, >> >> On 05/28/2018 02:46 PM, Joel Stanley wrote: >>> The ASPEED SoCs contain a single register that returns random data when >>> read. This models that register so that guests can use it. >>> >>> Signed-off-by: Joel Stanley >>> --- >>> hw/misc/aspeed_scu.c | 19 +++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c >>> index 5e6d5744eeca..8fa0cecf0fa1 100644 >>> --- a/hw/misc/aspeed_scu.c >>> +++ b/hw/misc/aspeed_scu.c >>> @@ -16,6 +16,7 @@ >>> #include "qapi/visitor.h" >>> #include "qemu/bitops.h" >>> #include "qemu/log.h" >>> +#include "crypto/random.h" >>> #include "trace.h" >>> >>> #define TO_REG(offset) ((offset) >> 2) >>> @@ -154,6 +155,18 @@ static const uint32_t >>> ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { >>> [BMC_DEV_ID] = 0x2402U >>> }; >>> >>> +static uint32_t aspeed_scu_get_random(void) >>> +{ >>> +Error *err = NULL; >>> +uint32_t num; >>> + >>> +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { >>> +error_report_err(err); >>> +} >>> + >>> +return num; >>> +} >>> + >>> static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) >>> { >>> AspeedSCUState *s = ASPEED_SCU(opaque); >>> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr >>> offset, unsigned size) >>> } >>> >>> switch (reg) { >>> +case RNG_DATA: >>> +return aspeed_scu_get_random(); >> >> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not. > > The RNG is enabled by default, and I didn't find any software that > disables it, but it can't hurt to have that check. I did some testing on hardware, and the rng still outputs a different number each time I ask for one regardless of the state of the enabled bit. How should we model that?
Re: [Qemu-devel] [PATCH 3/4] spapr: introduce a generic IRQ frontend to the machine
On Fri, 18 May 2018 18:44:04 +0200 Cédric Le Goaterwrote: > This proposal moves all the related IRQ routines of the sPAPR machine > behind a class interface to prepare for future changes in the IRQ > controller model. First of which is a reorganization of the IRQ number > space layout and a second, coming later, will be to integrate the > support for the new POWER9 XIVE IRQ controller. > > The new interface defines a set of fixed IRQ number ranges, for each > IRQ type, in which devices allocate the IRQ numbers they need > depending on a unique device index. Here is the layout : > > SPAPR_IRQ_IPI0x0/* 1 IRQ per CPU */ > SPAPR_IRQ_EPOW 0x1000 /* 1 IRQ per device */ > SPAPR_IRQ_HOTPLUG0x1001 /* 1 IRQ per device */ > SPAPR_IRQ_VIO0x1100 /* 1 IRQ per device */ > SPAPR_IRQ_PCI_LSI0x1200 /* 4 IRQs per device */ > SPAPR_IRQ_PCI_MSI0x1400 /* 1K IRQs per device */ > > The IPI range is reserved for future use when XIVE support > comes in. > > The routines of this interface encompass the previous needs and the > new ones and seem complex but the provided IRQ backend should > implement what we have today without any functional changes. > > Each device model is modified to take the new interface into account > using the IRQ range/type definitions and a device index. A part from > the VIO devices, lacking an id, the changes are relatively simple. > > Signed-off-by: Cédric Le Goater > --- > include/hw/ppc/spapr.h | 10 +- > include/hw/ppc/spapr_irq.h | 46 + > hw/ppc/spapr.c | 177 +- > hw/ppc/spapr_events.c | 7 +- > hw/ppc/spapr_irq.c | 233 > + > hw/ppc/spapr_pci.c | 21 +++- > hw/ppc/spapr_vio.c | 5 +- > hw/ppc/Makefile.objs | 2 +- > 8 files changed, 308 insertions(+), 193 deletions(-) > create mode 100644 include/hw/ppc/spapr_irq.h > create mode 100644 hw/ppc/spapr_irq.c > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 2cfdfdd67eaf..4eb212b16a51 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -3,10 +3,10 @@ > > #include "sysemu/dma.h" > #include "hw/boards.h" > -#include "hw/ppc/xics.h" > #include "hw/ppc/spapr_drc.h" > #include "hw/mem/pc-dimm.h" > #include "hw/ppc/spapr_ovec.h" > +#include "hw/ppc/spapr_irq.h" > > struct VIOsPAPRBus; > struct sPAPRPHBState; > @@ -104,6 +104,7 @@ struct sPAPRMachineClass { >unsigned n_dma, uint32_t *liobns, Error **errp); > sPAPRResizeHPT resize_hpt_default; > sPAPRCapabilities default_caps; > +sPAPRIrq *irq; > }; > > /** > @@ -773,13 +774,6 @@ int spapr_get_vcpu_id(PowerPCCPU *cpu); > void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp); > PowerPCCPU *spapr_find_cpu(int vcpu_id); > > -int spapr_irq_alloc(sPAPRMachineState *spapr, bool lsi, Error **errp); > -int spapr_irq_alloc_block(sPAPRMachineState *spapr, int num, bool lsi, > - bool align, Error **errp); > -void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num); > -qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq); > - > - > int spapr_caps_pre_load(void *opaque); > int spapr_caps_pre_save(void *opaque); > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > new file mode 100644 > index ..caf4c33d4cec > --- /dev/null > +++ b/include/hw/ppc/spapr_irq.h > @@ -0,0 +1,46 @@ > +/* > + * QEMU PowerPC sPAPR IRQ backend definitions > + * > + * Copyright (c) 2018, IBM Corporation. > + * > + * This code is licensed under the GPL version 2 or later. See the > + * COPYING file in the top-level directory. > + */ > +#ifndef HW_SPAPR_IRQ_H > +#define HW_SPAPR_IRQ_H > + > +#include "hw/ppc/xics.h" > + > +/* > + * IRQ ranges > + */ > +#define SPAPR_IRQ_IPI0x0 /* 1 IRQ per CPU */ > +#define SPAPR_IRQ_EPOW 0x1000 /* 1 IRQ per device */ > +#define SPAPR_IRQ_HOTPLUG0x1001 /* 1 IRQ per device */ > +#define SPAPR_IRQ_VIO0x1100 /* 1 IRQ per device */ > +#define SPAPR_IRQ_PCI_LSI0x1200 /* 4 IRQs per device */ > +#define SPAPR_IRQ_PCI_MSI0x1400 /* 1K IRQs per device covered by > + * a bitmap allocator */ > + > +typedef struct sPAPRIrq { > +uint32_tnr_irqs; > + > +void (*init)(sPAPRMachineState *spapr, Error **errp); > +int (*alloc)(sPAPRMachineState *spapr, uint32_t range, uint32_t index, > + Error **errp); > +int (*alloc_block)(sPAPRMachineState *spapr, uint32_t range, > + uint32_t index, int num, bool align, Error **errp); > +void (*free)(sPAPRMachineState *spapr, int irq, int num, Error **errp); > +qemu_irq (*qirq)(sPAPRMachineState *spapr, int irq); > +} sPAPRIrq; > + > +extern sPAPRIrq spapr_irq_default; > + >
[Qemu-devel] Cortex M0 emulation tasks
Hi, I took a look at what's required for ARM Cortex M0 emulation that we need for the micro:bit ARM board. The following notes are based on Appendix D3 of the ARMv6-M Architecture Reference Manual that Peter Maydell recommended. Several people can work on this since there are many smaller tasks. The general approach for each task: 1. Look at Appendix D3 to understand the architecture differences. 2. Look at QEMU source code (mainly target/arm/translate.c) to understand the current status. 3. Implement a ARMv6-M-specific code path, if necessary. 4. Implement a test case that exercises the instruction or register to prove it works. Before we can tackle these tasks a Cortex M0 CPU needs to be defined. Adding the Cortex M0 involves a new element in target/arm/cpu.c:arm_cpus[]. The CPU needs ARM_FEATURE_V6 and ARM_FEATURE_M. Once that is in place most of these tasks can be done independently and by multiple people. How to collaborate: Pick something you want to do from this list and reply-all to announce you are working on it. Tasks: D3.3 Instruction Support Each of the listed instructions needs to be audited. For example, "32-bit DMB, DSB and ISB barrier instructions" will not work because disas_thumb2_insn() won't run for ARM_FEATURE_V6 without ARM_FEATURE_THUMB2: /* The only 32 bit insn that's allowed for Thumb1 is the combined * BL/BLX prefix and suffix. */ if ((insn & 0xf800e800) != 0xf000e800) { ARCH(6T2); <-- this will fail on ARMv6-M } The 32-bit Thumb2 DMB, DSB, and ISB instructions need to be added as special cases for ARMv6-M. And there is a second check that will fail: case 3: /* Special control operations. */ ARCH(7); <-- this is too restrictive! op = (insn >> 4) & 0xf; switch (op) { case 2: /* clrex */ gen_clrex(s); <--- not supported on ARMv6-M break; case 4: /* dsb */ <--- supported on ARMv6-M case 5: /* dmb */ <--- supported on ARMv6-M tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); break; case 6: /* isb */ <--- supported on ARMv6-M /* We need to break the TB after this insn * to execute self-modifying code correctly * and also to take any pending interrupts * immediately. */ gen_goto_tb(s, 0, s->pc & ~1); break; default: goto illegal_op; } These instructions must be tested to prove they do not fault and behave properly. I recommend a similar approach to tests/boot-serial-test.c consisting of a tiny instruction sequence. D3.4 Programmer's model Lot's of details to check here. I haven't investigated it. Comments appreciated. D3.5 Memory model This can probably be ignored, it's very similar to ARMv7-M. D3.5.1 Alignment support ARMv7-M supports unaligned accesses but ARMv6-M does not. Check code to ensure alignment checks are being made. D3.5.2 Endian support Nothing to do here. D3.5.3 Exclusive access support Each of the listed instructions must be checked. For example, STREX is not supported ARMv6-M but QEMU seems to allow it. D3.5.4 Cache support Nothing to do here. D3.5.5 PMSA support I think we can ignore this. QEMU implements PMSA. It would be best to check if Cortex M0 wants PMSAv6. D3.6.1 Reserved registers in ARMv6-M Listed registers must be emulated "Read-as-Zero, Write Ignored" for ARMv6-M. This means guest memory load instructions produce 0 and store instructions have no effect. D3.6.2 General Fault Status Registers ARMv6-M has a coarser fault handling architecture, the code needs to be audited to check that HardFault is raised under all conditions. D3.6.3 System timer support No changes necessary. My interpretation is that SysTick is optional for Cortex M0 but the nRF51 implements it. It is mandatory for ARMv7-M, so let's just keep it. D3.6.4 NVIC support Some registers are reserved on ARMv6-M and there are fewer priority levels. D3.7 Debug support I think we can ignore this because debugging is handled by QEMU via the gdbstub. Stefan
Re: [Qemu-devel] [RFC v2 2/6] hw/arm/virt: Enable dynamic generation of guest RAM memory regions
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > Register ram_memory_region_init notifier to allocate memory region > from system memory. At this stage the commit message does not explain why you need a machine init done notifier. Also the commit title does not summarize the actual change. Thanks Eric > > Signed-off-by: Zhu Yijun> Signed-off-by: Shameer Kolothum > --- > hw/arm/virt.c | 28 ++-- > include/hw/arm/virt.h | 1 + > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 94dcb12..05fcb62 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1171,6 +1171,19 @@ void virt_machine_done(Notifier *notifier, void *data) > virt_build_smbios(vms); > } > > +static void virt_ram_memory_region_init(Notifier *notifier, void *data) > +{ > +MemoryRegion *sysmem = get_system_memory(); > +MemoryRegion *ram = g_new(MemoryRegion, 1); > +VirtMachineState *vms = container_of(notifier, VirtMachineState, > + ram_memory_region_init); > +MachineState *machine = MACHINE(vms); > + > +memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram", > + machine->ram_size); > +memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram); > +} > + > static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx) > { > uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; > @@ -1204,7 +1217,6 @@ static void machvirt_init(MachineState *machine) > MemoryRegion *sysmem = get_system_memory(); > MemoryRegion *secure_sysmem = NULL; > int n, virt_max_cpus; > -MemoryRegion *ram = g_new(MemoryRegion, 1); > bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); > > /* We can probe only here because during property set > @@ -1361,10 +1373,6 @@ static void machvirt_init(MachineState *machine) > fdt_add_timer_nodes(vms); > fdt_add_cpu_nodes(vms); > > -memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram", > - machine->ram_size); > -memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram); > - > create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); > > create_gic(vms, pic); > @@ -1405,15 +1413,23 @@ static void machvirt_init(MachineState *machine) > vms->bootinfo.loader_start = vms->memmap[VIRT_MEM].base; > vms->bootinfo.get_dtb = machvirt_dtb; > vms->bootinfo.firmware_loaded = firmware_loaded; > + > +/* Register notifiers. They are executed in registration reverse order */ > arm_load_kernel(ARM_CPU(first_cpu), >bootinfo); > > /* > * arm_load_kernel machine init done notifier registration must > * happen before the platform_bus_create call. In this latter, > * another notifier is registered which adds platform bus nodes. > - * Notifiers are executed in registration reverse order. > */ > create_platform_bus(vms, pic); > + > +/* > + * Register memory region notifier last as this has to be executed > + * first. > + */ > +vms->ram_memory_region_init.notify = virt_ram_memory_region_init; > +qemu_add_machine_init_done_notifier(>ram_memory_region_init); > } > > static bool virt_get_secure(Object *obj, Error **errp) > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index ba0c1a4..fc24f3a 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -91,6 +91,7 @@ typedef struct { > typedef struct { > MachineState parent; > Notifier machine_done; > +Notifier ram_memory_region_init; > FWCfgState *fw_cfg; > bool secure; > bool highmem; >
Re: [Qemu-devel] [RFC v2 0/6] hw/arm: Add support for non-contiguous iova regions
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > When the kernel reports valid iova ranges as non-contiguous, > memory should be allocated to Guest in such a way that > reserved regions(holes) are not visible by Guest. > > This series retrieves the valid iova ranges based on the new > proposed VFIO interface for kernel [1]. It then populates the > first 1GB ram as a non-pluggable dimm and rest as a pc-dimm if > the valid iova ranges are non-contiguous. Some general comments: - what are your plans with respect to VFIO device hot-plug handling? IIUC, at the moment, any collision between reserved regions induces by hot-plugged devices are not detected/handled. I understand mem hotplug is not supported on aarch64. Would you simply reject the vfio device hotplug. - IIUC any reserved window colliding with [400, 1GB] cold-plug RAM segment is show-stopper. How was this 1GB size chosen exactly? - Currently you create a single PC-DIMM node whereas several ones may be possible & necessary? Do you plan to support multiple PC-DIMMS node? - I have started looking at RAM extension on machvirt. I think adding support of PC-DIMMS through the qemu cmd line is something that we need to work on, in paralell. Thanks Eric > > Patch #3 of this series is loosely based on an earlier attempt > by Kwangwoo Lee to add hotplug/pc-dimm support to arm64[2] > > RFC v1[3] --> RFCv2 > -Based on new VFIO kernel interface > -Part of Mem modelled as pc-dimm > -Rebased to qemu 2.12.0 > > [1] https://lkml.org/lkml/2018/4/18/293 > [2] https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg04600.html > [3] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02412.html > > Shameer Kolothum (6): > hw/vfio: Retrieve valid iova ranges from kernel > hw/arm/virt: Enable dynamic generation of guest RAM memory regions > hw/arm/virt: Add pc-dimm mem hotplug framework > hw/arm: Changes required to accommodate non-contiguous DT mem nodes > hw/arm: ACPI SRAT changes to accommodate non-contiguous mem > hw/arm: Populate non-contiguous memory regions > > default-configs/aarch64-softmmu.mak | 1 + > hw/arm/boot.c | 91 ++--- > hw/arm/virt-acpi-build.c| 24 ++- > hw/arm/virt.c | 367 > +++- > hw/vfio/common.c| 108 ++- > include/hw/arm/arm.h| 12 ++ > include/hw/arm/virt.h | 3 + > include/hw/vfio/vfio-common.h | 7 + > linux-headers/linux/vfio.h | 23 +++ > 9 files changed, 589 insertions(+), 47 deletions(-) >
Re: [Qemu-devel] [RFC v2 3/6] hw/arm/virt: Add pc-dimm mem hotplug framework
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > This will be used in subsequent patches to model a chunk of > memory as pc-dimm(cold plug) if the valid iova regions are > non-contiguous. This is not yet a full hotplug support. Please can you give more details about this restriction? > > Signed-off-by: Shameer Kolothum> --- > default-configs/aarch64-softmmu.mak | 1 + > hw/arm/virt.c | 82 > + > include/hw/arm/virt.h | 2 + > 3 files changed, 85 insertions(+) > > diff --git a/default-configs/aarch64-softmmu.mak > b/default-configs/aarch64-softmmu.mak > index 9ddccf8..7a82ed8 100644 > --- a/default-configs/aarch64-softmmu.mak > +++ b/default-configs/aarch64-softmmu.mak > @@ -8,3 +8,4 @@ CONFIG_DDC=y > CONFIG_DPCD=y > CONFIG_XLNX_ZYNQMP=y > CONFIG_XLNX_ZYNQMP_ARM=y > +CONFIG_MEM_HOTPLUG=y > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 05fcb62..be3ad14 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1552,9 +1552,82 @@ static const CPUArchIdList > *virt_possible_cpu_arch_ids(MachineState *ms) > return ms->possible_cpus; > } > > +static void virt_dimm_plug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > +VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > +PCDIMMDevice *dimm = PC_DIMM(dev); > +PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); > +MemoryRegion *mr; > +uint64_t align; > +Error *local_err = NULL; > + > +mr = ddc->get_memory_region(dimm, _err); > +if (local_err) { > +goto out; > +} > + > +align = memory_region_get_alignment(mr); I see that on PC machine class they have an "enforce_aligned_dimm" option. Also looks memory_region_get_alignment(mr) can return 0. if (memory_region_get_alignment(mr) && pcmc->enforce_aligned_dimm) { > +pc_dimm_memory_plug(dev, >hotplug_memory, mr, align, _err); > +if (local_err) { > +goto out; > +} useless block and error_propagate does nothing if local_err is NULL so we are fine. > + > +out: > +error_propagate(errp, local_err); > +} > + > +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > +VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > +PCDIMMDevice *dimm = PC_DIMM(dev); > +PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); > +MemoryRegion *mr; > +Error *local_err = NULL; > + > +mr = ddc->get_memory_region(dimm, _err); don't you want to avoid executing the following if local_err? > +pc_dimm_memory_unplug(dev, >hotplug_memory, mr); needs a rebase on top of "pc-dimm: no need to pass the memory region" in master pc_dimm_memory_unplug we now have MemoryRegion *mr = ddc->get_memory_region(dimm, _abort); Thanks Eric > +object_unparent(OBJECT(dev)); > + > +error_propagate(errp, local_err); > +} > + > +static void virt_machinedevice_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > +virt_dimm_plug(hotplug_dev, dev, errp); > +} else { > +error_setg(errp, "device plug request for not supported device" > + " type: %s", object_get_typename(OBJECT(dev))); > +} > +} > + > +static void virt_machinedevice_unplug_cb(HotplugHandler *hotplug_dev, > +DeviceState *dev, Error **errp) > +{ > +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > +virt_dimm_unplug(hotplug_dev, dev, errp); > +} else { > +error_setg(errp, "device unplug for not supported device" > + " type: %s", object_get_typename(OBJECT(dev))); > +} > +} > + > +static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, > + DeviceState *dev) > +{ > +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > +return HOTPLUG_HANDLER(machine); > +} > +return NULL; > +} > + > static void virt_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > +HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); > > mc->init = machvirt_init; > /* Start max_cpus at the maximum QEMU supports. We'll further restrict > @@ -1573,6 +1646,11 @@ static void virt_machine_class_init(ObjectClass *oc, > void *data) > mc->cpu_index_to_instance_props = virt_cpu_index_to_props; > mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); > mc->get_default_cpu_node_id = virt_get_default_cpu_node_id; > + > +mc->get_hotplug_handler = virt_get_hotplug_handler; This requires a rebase on top of Igor's and David's series. Thanks Eric > +hc->plug = virt_machinedevice_plug_cb; > +hc->unplug = virt_machinedevice_unplug_cb; > + > } > > static const TypeInfo
Re: [Qemu-devel] [RFC v2 5/6] hw/arm: ACPI SRAT changes to accommodate non-contiguous mem
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > This is in preparation for the next patch where initial ram is split > into a non-pluggable chunk and a pc-dimm modeled mem if the vaild valid > iova regions are non-contiguous. > > Signed-off-by: Shameer Kolothum> --- > hw/arm/virt-acpi-build.c | 24 > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index c7c6a57..8d17b40 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -488,7 +488,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > AcpiSratProcessorGiccAffinity *core; > AcpiSratMemoryAffinity *numamem; > int i, srat_start; > -uint64_t mem_base; > +uint64_t mem_base, mem_sz, mem_len; > MachineClass *mc = MACHINE_GET_CLASS(vms); > const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(MACHINE(vms)); > > @@ -505,12 +505,28 @@ build_srat(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > core->flags = cpu_to_le32(1); > } > > -mem_base = vms->memmap[VIRT_MEM].base; > +mem_base = vms->bootinfo.loader_start; > +mem_sz = vms->bootinfo.loader_start; > for (i = 0; i < nb_numa_nodes; ++i) { > numamem = acpi_data_push(table_data, sizeof(*numamem)); > -build_srat_memory(numamem, mem_base, numa_info[i].node_mem, i, > +mem_len = MIN(numa_info[i].node_mem, mem_sz); same question as in previous patch Thanks Eric > +build_srat_memory(numamem, mem_base, mem_len, i, >MEM_AFFINITY_ENABLED); > -mem_base += numa_info[i].node_mem; > +mem_base += mem_len; > +mem_sz -= mem_len; > +if (!mem_sz) { > +break; > +} > +} > + > +/* Create table for initial pc-dimm ram, if any */ > +if (vms->bootinfo.dimm_mem) { > +numamem = acpi_data_push(table_data, sizeof(*numamem)); > +build_srat_memory(numamem, vms->bootinfo.dimm_mem->base, > + vms->bootinfo.dimm_mem->size, > + vms->bootinfo.dimm_mem->node, > + MEM_AFFINITY_ENABLED); > + > } > > build_header(linker, table_data, (void *)(table_data->data + srat_start), >
Re: [Qemu-devel] [RFC v2 4/6] hw/arm: Changes required to accommodate non-contiguous DT mem nodes
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > This makes changes to the DT mem node creation such that its easier > to add non-contiguous mem modeled as non-pluggable and a pc-dimm > mem later. See comments below. I think you should augment the description here with what the patch exactly adds: - a new helper function - a new dimm node? and if possible split functional changes into separate patches? > > Signed-off-by: Shameer Kolothum> --- > hw/arm/boot.c| 91 > > include/hw/arm/arm.h | 12 +++ > 2 files changed, 75 insertions(+), 28 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 26184bc..73db0aa 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -486,6 +486,27 @@ static void fdt_add_psci_node(void *fdt) > qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn); > } > > +static char *create_memory_fdt(void *fdt, uint32_t acells, hwaddr mem_base, > + uint32_t scells, hwaddr mem_len) Other dt node creation functions are named fdt_add_*_node > +{ > +char *nodename = NULL; > +int rc; > + > +nodename = g_strdup_printf("/memory@%" PRIx64, mem_base); > +qemu_fdt_add_subnode(fdt, nodename); > +qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); > +rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", acells, mem_base, > + scells, mem_len); > +if (rc < 0) { > +fprintf(stderr, "couldn't set %s/reg\n", nodename); > +g_free(nodename); > +return NULL; > +} > + > +return nodename; > +} > + > + > /** > * load_dtb() - load a device tree binary image into memory > * @addr: the address to load the image at > @@ -567,50 +588,64 @@ static int load_dtb(hwaddr addr, const struct > arm_boot_info *binfo, > goto fail; > } > > +/* > + * Turn the /memory node created before into a NOP node, then create > + * /memory@addr nodes for all numa nodes respectively. > + */ > +qemu_fdt_nop_node(fdt, "/memory"); I don't really understand why this gets moved outside of the if (nb_numa_nodes > 0) { check. Also the comment above mention numa nodes whereas it are not necessarily in the numa case anymore. > + > if (nb_numa_nodes > 0) { > -/* > - * Turn the /memory node created before into a NOP node, then create > - * /memory@addr nodes for all numa nodes respectively. > - */ > -qemu_fdt_nop_node(fdt, "/memory"); > +hwaddr mem_sz; > + > mem_base = binfo->loader_start; > +mem_sz = binfo->ram_size; > for (i = 0; i < nb_numa_nodes; i++) { > -mem_len = numa_info[i].node_mem; > -nodename = g_strdup_printf("/memory@%" PRIx64, mem_base); > -qemu_fdt_add_subnode(fdt, nodename); > -qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); > -rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", > - acells, mem_base, > +mem_len = MIN(numa_info[i].node_mem, mem_sz); I fail to understand how this change relates to the topic of this patch. If this adds a consistency check, this may be put in another patch? > + > +nodename = create_memory_fdt(fdt, acells, mem_base, >scells, mem_len); You could simplify the review by just introducing the new dt node creation function in a 1st patch and then introduce the changes to support non contiguous DT mem nodes. > -if (rc < 0) { > -fprintf(stderr, "couldn't set %s/reg for node %d\n", > nodename, > -i); > +if (!nodename) { > goto fail; > } > > qemu_fdt_setprop_cell(fdt, nodename, "numa-node-id", i); > -mem_base += mem_len; > g_free(nodename); > +mem_base += mem_len; > +mem_sz -= mem_len; > +if (!mem_sz) { > +break; So what does it mean practically if we break here. Not all the num nodes will function? Outputting a mesg for the end user may be useful. > + } > } > -} else { > -Error *err = NULL; > > -rc = fdt_path_offset(fdt, "/memory"); > -if (rc < 0) { > -qemu_fdt_add_subnode(fdt, "/memory"); > -} > +/* Create the node for initial pc-dimm ram, if any */ > +if (binfo->dimm_mem) { > > -if (!qemu_fdt_getprop(fdt, "/memory", "device_type", NULL, )) { > -qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory"); > +nodename = create_memory_fdt(fdt, acells, binfo->dimm_mem->base, > + scells, binfo->dimm_mem->size); > +if (!nodename) { > +
Re: [Qemu-devel] [RFC v2 6/6] hw/arm: Populate non-contiguous memory regions
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > In case valid iova regions are non-contiguous, split the > RAM mem into a 1GB non-pluggable dimm and remaining as a > single pc-dimm mem. Please can you explain where does this split come from? Currently we have 254 GB non pluggable RAM. I read the discussion started with Drew on RFC v1 where he explained we cannot change the RAM base without crashing the FW. However we should at least document this 1GB split. > > Signed-off-by: Shameer Kolothum> --- > hw/arm/virt.c | 261 > -- > 1 file changed, 256 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index be3ad14..562e389 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -58,6 +58,12 @@ > #include "hw/smbios/smbios.h" > #include "qapi/visitor.h" > #include "standard-headers/linux/input.h" > +#include "hw/vfio/vfio-common.h" > +#include "qemu/config-file.h" > +#include "monitor/qdev.h" > +#include "qom/object_interfaces.h" > +#include "qapi/qmp/qdict.h" > +#include "qemu/option.h" The comment at the beginning of virt.c would need to be reworked with your changes. > > #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \ > static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ > @@ -110,7 +116,10 @@ static ARMPlatformBusSystemParams platform_bus_params; > * terabyte of physical address space.) > */ > #define RAMLIMIT_GB 255 > -#define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024) > +#define SZ_1G (1024ULL * 1024 * 1024) > +#define RAMLIMIT_BYTES (RAMLIMIT_GB * SZ_1G) > + > +#define ALIGN_1G (1ULL << 30) > > /* Addresses and sizes of our components. > * 0..128MB is space for a flash device so we can run bootrom code such as > UEFI. > @@ -1171,6 +1180,236 @@ void virt_machine_done(Notifier *notifier, void *data) > virt_build_smbios(vms); > } > > +static void free_iova_copy(struct vfio_iova_head *iova_copy) > +{ > +VFIOIovaRange *iova, *tmp; > + > +QLIST_FOREACH_SAFE(iova, iova_copy, next, tmp) { > +QLIST_REMOVE(iova, next); > +g_free(iova); > +} > +} > + > +static void get_iova_copy(struct vfio_iova_head *iova_copy) > +{ > +VFIOIovaRange *iova, *new, *prev_iova = NULL; > + > +QLIST_FOREACH(iova, _iova_regions, next) { > +new = g_malloc0(sizeof(*iova)); > +new->start = iova->start; > +new->end = iova->end; > + > +if (prev_iova) { > +QLIST_INSERT_AFTER(prev_iova, new, next); > +} else { > +QLIST_INSERT_HEAD(iova_copy, new, next); > +} > +prev_iova = new; > +} > +} > + > +static hwaddr find_memory_chunk(VirtMachineState *vms, > + struct vfio_iova_head *iova_copy, > + hwaddr req_size, bool pcdimm) > +{ > +VFIOIovaRange *iova, *tmp; > +hwaddr new_start, new_size, sz_align; > +hwaddr virt_start = vms->memmap[VIRT_MEM].base; > +hwaddr addr_align = ALIGN_1G; /* Set to max ARM64 hugepage size */ > + > +/* Size alignment */ > +sz_align = (pcdimm) ? MAX(TARGET_PAGE_SIZE, QEMU_VMALLOC_ALIGN) : > + TARGET_PAGE_SIZE; > + > +QLIST_FOREACH_SAFE(iova, iova_copy, next, tmp) { > +if (virt_start >= iova->end) { > +continue; > +} > + > +/* Align addr */ > +new_start = ROUND_UP(MAX(virt_start, iova->start), addr_align); > +if (new_start >= iova->end) { > +continue; > +} > + > +if (req_size > iova->end - new_start + 1) { > +continue; > +} don't you want to check directly with new_size? > + > +/* > + * Check the region can hold any size alignment requirement. > + */ > +new_size = QEMU_ALIGN_UP(req_size, sz_align); > + > +if ((new_start + new_size - 1 > iova->end) || > + (new_start + new_size >= virt_start + RAMLIMIT_BYTES)) { > +continue; > +} > + > +/* > + * Modify the iova list entry for non pc-dimm case so that it > + * is not used again for pc-dimm allocation. > + */ > +if (!pcdimm) { > +if (new_size - req_size) { I don't get this check, Don't you want to check the node's range is fully consumed and in the positive remove the node? (new_size != iova->end - iova-> start + 1) > +iova->start = new_start + new_size; > +} else { > +QLIST_REMOVE(iova, next); > +} > +} > +return new_start; > +} > + > +return -1; > +} > + > +static void update_memory_regions(VirtMachineState *vms) > +{ > +hwaddr addr; > +VFIOIovaRange *iova; > +MachineState *machine = MACHINE(vms); > +hwaddr virt_start = vms->memmap[VIRT_MEM].base; > +hwaddr req_size, ram_size = machine->ram_size; > +struct
Re: [Qemu-devel] [RFC v2 1/6] hw/vfio: Retrieve valid iova ranges from kernel
Hi Shameer, On 05/16/2018 05:20 PM, Shameer Kolothum wrote: > This makes use of the newly introduced iova cap chains added > to the type1 VFIO_IOMMU_GET_INFO ioctl. > > The retrieved iova info is stored in a list for later use. > > Signed-off-by: Shameer Kolothum> --- > hw/vfio/common.c | 108 > +++--- > include/hw/vfio/vfio-common.h | 7 +++ > linux-headers/linux/vfio.h| 23 + > 3 files changed, 132 insertions(+), 6 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 07ffa0b..94d7b24 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -40,6 +40,8 @@ struct vfio_group_head vfio_group_list = > QLIST_HEAD_INITIALIZER(vfio_group_list); > struct vfio_as_head vfio_address_spaces = > QLIST_HEAD_INITIALIZER(vfio_address_spaces); > +struct vfio_iova_head vfio_iova_regions = > +QLIST_HEAD_INITIALIZER(vfio_iova_regions); > > #ifdef CONFIG_KVM > /* > @@ -1030,6 +1032,85 @@ static void vfio_put_address_space(VFIOAddressSpace > *space) > } > } > > +static void vfio_iommu_get_iova_ranges(struct vfio_iommu_type1_info *info) > +{ > +struct vfio_info_cap_header *hdr; > +struct vfio_iommu_type1_info_cap_iova_range *cap_iova; > +VFIOIovaRange *iova, *tmp, *prev = NULL; nit: s/iova/iova_range? > +void *ptr = info; > +bool found = false; > +int i; > + > +if (!(info->flags & VFIO_IOMMU_INFO_CAPS)) { > +return; > +} > + > +for (hdr = ptr + info->cap_offset; hdr != ptr; hdr = ptr + hdr->next) { > +if (hdr->id == VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE) { > +found = true; > +break; > +} > +} > + > +if (!found) { > +return; > +} > + > +/* purge the current iova list, if any */ > +QLIST_FOREACH_SAFE(iova, _iova_regions, next, tmp) { > +QLIST_REMOVE(iova, next); > +g_free(iova); > +} > + > +cap_iova = container_of(hdr, struct vfio_iommu_type1_info_cap_iova_range, > +header); > + > +/* populate the list */ > +for (i = 0; i < cap_iova->nr_iovas; i++) { > +iova = g_malloc0(sizeof(*iova)); nit: g_new0 is preferred > +iova->start = cap_iova->iova_ranges[i].start; > +iova->end = cap_iova->iova_ranges[i].end; > + > +if (prev) { > +QLIST_INSERT_AFTER(prev, iova, next); > +} else { > +QLIST_INSERT_HEAD(_iova_regions, iova, next); > +} > +prev = iova; > +} > + > +return; > +} > + > +static int vfio_get_iommu_info(VFIOContainer *container, > + struct vfio_iommu_type1_info **info) > +{ > + > +size_t argsz = sizeof(struct vfio_iommu_type1_info); > + > + > +*info = g_malloc0(argsz); > + > +retry: > +(*info)->argsz = argsz; > + > +if (ioctl(container->fd, VFIO_IOMMU_GET_INFO, *info)) { > +g_free(*info); > +*info = NULL; > +return -errno; > +} > + > +if (((*info)->argsz > argsz)) { > +argsz = (*info)->argsz; > +*info = g_realloc(*info, argsz); > +goto retry; > +} > + > +vfio_iommu_get_iova_ranges(*info); > + > +return 0; > +} > + > static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, >Error **errp) > { > @@ -1044,6 +1125,15 @@ static int vfio_connect_container(VFIOGroup *group, > AddressSpace *as, > group->container = container; > QLIST_INSERT_HEAD(>group_list, group, container_next); > vfio_kvm_device_add_group(group); > + > +/* New group might change the valid iovas. Get the updated list > */ > +if ((container->iommu_type == VFIO_TYPE1_IOMMU) || > +(container->iommu_type == VFIO_TYPE1v2_IOMMU)) { > +struct vfio_iommu_type1_info *info; > + > +vfio_get_iommu_info(container, ); > +g_free(info); > +} > return 0; > } > } > @@ -1071,7 +1161,7 @@ static int vfio_connect_container(VFIOGroup *group, > AddressSpace *as, > if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) || > ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) { > bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU); > -struct vfio_iommu_type1_info info; > +struct vfio_iommu_type1_info *info; > > ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, ); > if (ret) { > @@ -1095,14 +1185,14 @@ static int vfio_connect_container(VFIOGroup *group, > AddressSpace *as, > * existing Type1 IOMMUs generally support any IOVA we're > * going to actually try in practice. > */ > -info.argsz = sizeof(info); > -ret = ioctl(fd, VFIO_IOMMU_GET_INFO, ); > +ret = vfio_get_iommu_info(container, ); > /* Ignore errors */ > -if
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
On 28 May 2018 at 23:17, Cédric Le Goaterwrote: > Hello Joel, > > On 05/28/2018 02:46 PM, Joel Stanley wrote: >> The ASPEED SoCs contain a single register that returns random data when >> read. This models that register so that guests can use it. >> >> Signed-off-by: Joel Stanley >> --- >> hw/misc/aspeed_scu.c | 19 +++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c >> index 5e6d5744eeca..8fa0cecf0fa1 100644 >> --- a/hw/misc/aspeed_scu.c >> +++ b/hw/misc/aspeed_scu.c >> @@ -16,6 +16,7 @@ >> #include "qapi/visitor.h" >> #include "qemu/bitops.h" >> #include "qemu/log.h" >> +#include "crypto/random.h" >> #include "trace.h" >> >> #define TO_REG(offset) ((offset) >> 2) >> @@ -154,6 +155,18 @@ static const uint32_t >> ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { >> [BMC_DEV_ID] = 0x2402U >> }; >> >> +static uint32_t aspeed_scu_get_random(void) >> +{ >> +Error *err = NULL; >> +uint32_t num; >> + >> +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { >> +error_report_err(err); >> +} >> + >> +return num; >> +} >> + >> static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) >> { >> AspeedSCUState *s = ASPEED_SCU(opaque); >> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr >> offset, unsigned size) >> } >> >> switch (reg) { >> +case RNG_DATA: >> +return aspeed_scu_get_random(); > > may be we could test bit 1 of RNG_CTRL to check if it is enabled or not. The RNG is enabled by default, and I didn't find any software that disables it, but it can't hurt to have that check. I'll send a v2 with that check. > >> +break; >> case WAKEUP_EN: >> qemu_log_mask(LOG_GUEST_ERROR, >>"%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", >> @@ -287,6 +303,9 @@ static void aspeed_scu_realize(DeviceState *dev, Error >> **errp) >>TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE); >> >> sysbus_init_mmio(sbd, >iomem); >> + >> +if (qcrypto_random_init(errp)) >> +return; >> } > > Isn't this routine called from main() already ? It is indirectly called, yes. I'll remove this. Cheers, Joel
Re: [Qemu-devel] [PULL 20/37] qcow2: Give the refcount cache the minimum possible size by default
On Mon 28 May 2018 03:49:07 PM CEST, Peter Maydell wrote: > On 28 May 2018 at 09:58, Alberto Garciawrote: >> On Mon 28 May 2018 10:38:55 AM CEST, Kevin Wolf wrote: > +if (!refcount_cache_size_set) { > +*refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE * > s->cluster_size; ...but in the else clause down here, we don't have the cast, and Coverity complains that we evaluate a 32-bit result and then put it in a 64-bit variable. Should this have the (uint64_t) cast as well ? >> >> I suppose that's not because we put a 32-bit result in a 64-bit >> variable, but because it could theoretically overflow (if >> s->cluster_size > INT32_MAX / 4). > > Well, coverity warns because it's one of those things that could be > correct code, or could be the author tripping over one of C's > less-than-obvious traps. 32x32->32 multiplies are just as susceptible > to overflow, but the heuristic Coverity uses is "calculated a 32-bit > multiply result and put it in a 64-bit variable", on the assumption > that the type of the destination implies that whatever you're > calculating could well be that big, and "truncate the result of my > multiply even though it would fit in the destination" is a bit > unexpected. Coverity's wrong quite a bit on this one, naturally, but > it's usually easier to shut it up on the wrong guesses for the benefit > of the times when it turns out to be right. Yes, it's a fair warning. I'll send a patch. Berto
Re: [Qemu-devel] [PATCH 02/13] 9p: Avoid warning if FS_IOC_GETVERSION is not defined
On Sat, 26 May 2018 01:23:04 -0400 k...@juliacomputing.com wrote: > From: Keno Fischer> > Signed-off-by: Keno Fischer > --- > hw/9pfs/9p-local.c | 39 +++ > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index f6c7526..7592f8d 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -1375,10 +1375,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath > *dir, > return ret; > } > > +#ifdef FS_IOC_GETVERSION > static int local_ioc_getversion(FsContext *ctx, V9fsPath *path, > mode_t st_mode, uint64_t *st_gen) > { > -#ifdef FS_IOC_GETVERSION > int err; > V9fsFidOpenState fid_open; > > @@ -1397,15 +1397,11 @@ static int local_ioc_getversion(FsContext *ctx, > V9fsPath *path, > err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen); > local_close(ctx, _open); > return err; > -#else > -errno = ENOTTY; > -return -1; > -#endif > } > +#endif > > static int local_init(FsContext *ctx, Error **errp) > { > -struct statfs stbuf; > LocalData *data = g_malloc(sizeof(*data)); > > data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY); > @@ -1415,20 +1411,23 @@ static int local_init(FsContext *ctx, Error **errp) > } > > #ifdef FS_IOC_GETVERSION > -/* > - * use ioc_getversion only if the ioctl is definied > - */ > -if (fstatfs(data->mountfd, ) < 0) { > -close_preserve_errno(data->mountfd); Hmm... I now realize that this path doesn't set errp, which means that we could possibly fail the device realization without reporting any error to the caller. Could you please fix this in a preparatory patch ? > -goto err; > -} > -switch (stbuf.f_type) { > -case EXT2_SUPER_MAGIC: > -case BTRFS_SUPER_MAGIC: > -case REISERFS_SUPER_MAGIC: > -case XFS_SUPER_MAGIC: > -ctx->exops.get_st_gen = local_ioc_getversion; > -break; > +{ > +struct statfs stbuf; > +/* > +* use ioc_getversion only if the ioctl is definied > +*/ > +if (fstatfs(data->mountfd, ) < 0) { > +close_preserve_errno(data->mountfd); > +goto err; > +} > +switch (stbuf.f_type) { > +case EXT2_SUPER_MAGIC: > +case BTRFS_SUPER_MAGIC: > +case REISERFS_SUPER_MAGIC: > +case XFS_SUPER_MAGIC: > +ctx->exops.get_st_gen = local_ioc_getversion; > +break; > +} Please move this to a separate local_ioc_getversion_init() function, that would be empty if FS_IOC_GETVERSION is not defined. > } > #endif >
Re: [Qemu-devel] [PULL 20/37] qcow2: Give the refcount cache the minimum possible size by default
On 28 May 2018 at 09:58, Alberto Garciawrote: > On Mon 28 May 2018 10:38:55 AM CEST, Kevin Wolf wrote: >>> > +if (!refcount_cache_size_set) { >>> > +*refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE * >>> > s->cluster_size; >>> >>> ...but in the else clause down here, we don't have the cast, and >>> Coverity complains that we evaluate a 32-bit result and then put it >>> in a 64-bit variable. Should this have the (uint64_t) cast as well ? > > I suppose that's not because we put a 32-bit result in a 64-bit > variable, but because it could theoretically overflow (if > s->cluster_size > INT32_MAX / 4). Well, coverity warns because it's one of those things that could be correct code, or could be the author tripping over one of C's less-than-obvious traps. 32x32->32 multiplies are just as susceptible to overflow, but the heuristic Coverity uses is "calculated a 32-bit multiply result and put it in a 64-bit variable", on the assumption that the type of the destination implies that whatever you're calculating could well be that big, and "truncate the result of my multiply even though it would fit in the destination" is a bit unexpected. Coverity's wrong quite a bit on this one, naturally, but it's usually easier to shut it up on the wrong guesses for the benefit of the times when it turns out to be right. thanks -- PMM
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Hello Joel, On 05/28/2018 02:46 PM, Joel Stanley wrote: > The ASPEED SoCs contain a single register that returns random data when > read. This models that register so that guests can use it. > > Signed-off-by: Joel Stanley> --- > hw/misc/aspeed_scu.c | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index 5e6d5744eeca..8fa0cecf0fa1 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -16,6 +16,7 @@ > #include "qapi/visitor.h" > #include "qemu/bitops.h" > #include "qemu/log.h" > +#include "crypto/random.h" > #include "trace.h" > > #define TO_REG(offset) ((offset) >> 2) > @@ -154,6 +155,18 @@ static const uint32_t > ast2500_a1_resets[ASPEED_SCU_NR_REGS] = { > [BMC_DEV_ID] = 0x2402U > }; > > +static uint32_t aspeed_scu_get_random(void) > +{ > +Error *err = NULL; > +uint32_t num; > + > +if (qcrypto_random_bytes((uint8_t *), sizeof(num), )) { > +error_report_err(err); > +} > + > +return num; > +} > + > static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) > { > AspeedSCUState *s = ASPEED_SCU(opaque); > @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr > offset, unsigned size) > } > > switch (reg) { > +case RNG_DATA: > +return aspeed_scu_get_random(); may be we could test bit 1 of RNG_CTRL to check if it is enabled or not. > +break; > case WAKEUP_EN: > qemu_log_mask(LOG_GUEST_ERROR, >"%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", > @@ -287,6 +303,9 @@ static void aspeed_scu_realize(DeviceState *dev, Error > **errp) >TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE); > > sysbus_init_mmio(sbd, >iomem); > + > +if (qcrypto_random_init(errp)) > +return; > } Isn't this routine called from main() already ? C. > > static const VMStateDescription vmstate_aspeed_scu = { >
Re: [Qemu-devel] What is the best git-way to add a new board?
On 28 May 2018 at 06:02, Philippe Mathieu-Daudéwrote: > 1/ how RISC-V boards got merged > > Add devices individually, > finally add the board and default-configs/Makefile rules at once. > > PRO: commits are easier to cherry-pick/rebase > CON: you can not test a single patch until applying the whole series > harder (less interest) to write qtests I don't think this one has any real benefits over 1bis, below... > 1bis/ example of the Smartfusion2 board > > Variant of 1/, but also modify the default-configs/Makefile to > build each device. > > PRO: single patch can be test without much troubles, > unit/acceptance tests can be run without applying the whole series, > different people can work in parallel. > CON: no integration test until the last 'board' commit > > > 2/ Reverse Polish testing > > Add a board stub (all devices as UnimplementedDevice) and Makefile > rule, then for each new device, replace the unimp board entry, > modify the default-configs/Makefile, > add unit/acceptance tests > > PRO: you can add an integration test since the first board commit :) >(enforcing Test Driven Development), > allow unpaid contributor to slowly push his work upstream >instead of painful rebases or his work lost by change of >interest or worth issues. > CON: not acceptable by upstream except with good integration test > integration test might not be upstream-able due to license >incompatibilities (JTAG-extracted firmwares...) I usually go with some mix of 1 and 2, so for the first patchset upstream I have a board model and a few devices and the rest are stubbed out, but within that series I put the device models first and the board model last. While I'm doing the development before sending I might well have started with more stubs than the final version I send out has. It's also worth distinguishing between the shape of the commits you send upstream (which should be optimized for code review, for being a coherent but not oversized patchset, for not breaking compilation midway through the series, and so on), and how you work on them before you send them out. You don't have to start writing code with the first patch in the set :-) thanks -- PMM
Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()
On 05/28/2018 02:09 PM, Greg Kurz wrote: > On Mon, 28 May 2018 11:20:36 +0200 > Cédric Le Goaterwrote: > >> On 05/28/2018 09:18 AM, Thomas Huth wrote: >>> On 28.05.2018 09:06, Cédric Le Goater wrote: On 05/28/2018 08:17 AM, Thomas Huth wrote: > On 25.05.2018 16:02, Greg Kurz wrote: >> On Fri, 18 May 2018 18:44:02 +0200 >> Cédric Le Goater wrote: >> >>> This IRQ number hint can possibly be used by the VIO devices if the >>> "irq" property is defined on the command line but it seems it is never >>> the case. It is not used in libvirt for instance. So, let's remove it >>> to simplify future changes. >>> >> >> Setting an irq manually looks a bit anachronistic. I doubt anyone would >> do that nowadays, and the patch does a nice cleanup. So this looks like >> a good idea. > [...] >>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c >>> index 472dd6f33a96..cc064f64fccf 100644 >>> --- a/hw/ppc/spapr_vio.c >>> +++ b/hw/ppc/spapr_vio.c >>> @@ -455,7 +455,7 @@ static void spapr_vio_busdev_realize(DeviceState >>> *qdev, Error **errp) >>> dev->qdev.id = id; >>> } >>> >>> -dev->irq = spapr_irq_alloc(spapr, dev->irq, false, _err); >>> +dev->irq = spapr_irq_alloc(spapr, false, _err); >> >> Silently breaking "irq" like this looks wrong. I'd rather officially >> remove >> it first (ie, kill spapr_vio_props, -5 lines in spapr_vio.c). >> >> Of course, this raises the question of interface deprecation, and it >> should >> theoretically follow the process described at: >> >> https://wiki.qemu.org/Features/LegacyRemoval#Rules_for_removing_an_interface >> >> Cc'ing Thomas, our Chief Deprecation Officer, for insights :) > > The property is a public interface. Just because it's not used by > libvirt does not mean that nobody is using it. So yes, please follow the > rules and mark it as deprecated first for two release, before you really > remove it. This "irq" property is a problem to introduce a new static layout of IRQ numbers. It is in complete opposition. Can we keep it as it is for old pseries machine (settable) and ignore it for newer ? Would that be fine ? >>> >>> I think that would be fine, too. You likely need to keep the settable >>> IRQs around for the old machines anyway, to make sure that migration of >>> the old machine types still works, right? >> >> Yes, that is the goal of patch 3. It introduces a common sPAPR IRQ frontend, >> the first backend being the current one. >> > > If we keep "irq" but we ignore it with newer machine types, we should at > least print a warning then IMHO. May be we can deprecate at the same time. I will take a look. Thanks, C.
[Qemu-devel] [Bug 1773753] Re: virsh managed save fails with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc
followed by further attempts saves the domains as reported but issue still same. #virsh managedsave avocado-vt-vm1 Domain avocado-vt-vm1 state saved by libvirt # virsh start avocado-vt-vm1 hung # virsh list --all IdName State 98avocado-vt-vm1 paused I tried restarting libvirt, after which guest goes to shutoff state, with reason as crash in the qemu log # service libvirtd restart Redirecting to /bin/systemctl restart libvirtd.service # virsh list --all IdName State - avocado-vt-vm1 shut off 2018-05-28 12:59:46.748+: starting up libvirt version: 4.4.0, package: 1.fc28 (Unknown, 2018-05-28-03:15:39, 9.40.192.86), qemu version: 2.12.50v2.12.0-813-g5a5c383b13-dirty, kernel: 4.17.0-rc5-00069-g3acf4e395260, hostname: 9.40.192.86 LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin QEMU_AUDIO_DRV=none /usr/share/avocado-plugins-vt/bin/qemu -name guest=avocado-vt-vm1,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-97-avocado-vt-vm1/master-key.aes -machine pseries-2.13,accel=kvm,usb=off,dump-guest-core=off -m 1024 -realtime mlock=off -smp 2,maxcpus=4,sockets=4,cores=1,threads=1 -uuid ba3012d5-3244-47d9-bedc-0b60821f7cd1 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-97-avocado-vt-vm1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -kernel /home/kvmci/linux/vmlinux -append 'root=/dev/sda2 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug' -device qemu-xhci,id=usb,bus=pci.0,addr=0x3 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/var/lib/avocado/data/avocado-vt/images/jeos-27-ppc64le.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -netdev tap,fd=30,id=hostnet0,vhost=on,vhostfd=32 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:3d:3e:3f,bus=pci.0,addr=0x1 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,id=serial0,reg=0x3000 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/avocado-vt-vm1-guest.agent,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -sandbox off -msg timestamp=on 2018-05-28T12:59:46.826738Z qemu: -chardev pty,id=charserial0: char device redirected to /dev/pts/3 (label charserial0) 2018-05-28 13:00:52.948+: shutting down, reason=saved 2018-05-28T13:00:52.950802Z qemu: terminating on signal 15 from pid 41456 (/usr/sbin/libvirtd) 2018-05-28 13:01:00.467+: starting up libvirt version: 4.4.0, package: 1.fc28 (Unknown, 2018-05-28-03:15:39, 9.40.192.86), qemu version: 2.12.50v2.12.0-813-g5a5c383b13-dirty, kernel: 4.17.0-rc5-00069-g3acf4e395260, hostname: 9.40.192.86 LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin QEMU_AUDIO_DRV=none /usr/share/avocado-plugins-vt/bin/qemu -name guest=avocado-vt-vm1,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-98-avocado-vt-vm1/master-key.aes -machine pseries-2.13,accel=kvm,usb=off,dump-guest-core=off -m 1024 -realtime mlock=off -smp 2,maxcpus=4,sockets=4,cores=1,threads=1 -uuid ba3012d5-3244-47d9-bedc-0b60821f7cd1 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-98-avocado-vt-vm1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -kernel /home/kvmci/linux/vmlinux -append 'root=/dev/sda2 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug' -device qemu-xhci,id=usb,bus=pci.0,addr=0x3 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/var/lib/avocado/data/avocado-vt/images/jeos-27-ppc64le.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -netdev tap,fd=31,id=hostnet0,vhost=on,vhostfd=33 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:3d:3e:3f,bus=pci.0,addr=0x1 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,id=serial0,reg=0x3000 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/avocado-vt-vm1-guest.agent,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -incoming defer -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -sandbox
Re: [Qemu-devel] [PATCH 42/42] qemu-iotests: Test job-* with block jobs
On 2018-05-17 15:50, Kevin Wolf wrote: > Am 15.05.2018 um 01:44 hat Max Reitz geschrieben: >> On 2018-05-09 18:26, Kevin Wolf wrote: >>> This adds a test case that tests the new job-* QMP commands with >>> mirror and backup block jobs. >>> >>> Signed-off-by: Kevin Wolf>>> --- >>> tests/qemu-iotests/219 | 201 >>> tests/qemu-iotests/219.out | 327 >>> + >>> tests/qemu-iotests/group | 1 + >>> 3 files changed, 529 insertions(+) >>> create mode 100755 tests/qemu-iotests/219 >>> create mode 100644 tests/qemu-iotests/219.out > >>> +Pause/resume in READY >>> +=== Testing block-job-pause/block-job-resume === >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'standby', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'standby', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'ready', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'ready', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +=== Testing block-job-pause/job-resume === >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'standby', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'standby', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'ready', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'ready', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +=== Testing job-pause/block-job-resume === >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'standby', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'standby', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'ready', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'ready', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +=== Testing job-pause/job-resume === >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'standby', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'standby', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >>> +{u'return': {}} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'ready', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'return': [{u'status': u'ready', u'current-progress': 4194304, >>> u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]} >> >> This is really, really mean. Don't you have any compassion with the >> poor little job that just wants to have Feierabend? >> >> It worked so hard and it's always on standby and instantly ready when >> you need it. Yet, you keep it hanging. That's not nice. > > If you just mean that I do some testing with the poor job before I > complete it, then I'm afraid the job will have to suffer this. > > But if you have a more serious concern, I don't see it. Isn't the job > properly completed in the end, as the following lines show? No, I was just making fun of the standby <-> ready transition. :-) Max >>> +{u'error': {u'class': u'GenericError', u'desc': u"Job 'job0' in state >>> 'ready' cannot accept command verb 'finalize'"}} >>> +{u'error': {u'class': u'GenericError', u'desc': u"Job 'job0' in state >>> 'ready' cannot accept command verb 'dismiss'"}} >>> +{u'error': {u'class': u'GenericError', u'desc': u"Job 'job0' in state >>> 'ready' cannot accept command verb 'finalize'"}} >>> +{u'error': {u'class': u'GenericError', u'desc': u"Job 'job0' in state >>> 'ready' cannot accept command verb 'dismiss'"}} >>> +{u'return': {}} >>> + >>> +Waiting for PENDING state... >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'waiting', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'pending', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'} >>> +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': >>> {u'status': u'concluded',
Re: [Qemu-devel] [PATCH 13/13] 9p: darwin: configure: Allow VirtFS on Darwin
On Sat, 26 May 2018 01:23:15 -0400 k...@juliacomputing.com wrote: > From: Keno Fischer> > Signed-off-by: Keno Fischer > --- > Makefile.objs | 1 + > configure | 23 +++ > 2 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/Makefile.objs b/Makefile.objs > index c6c3554..a2245c9 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -104,6 +104,7 @@ common-obj-$(CONFIG_WIN32) += os-win32.o > common-obj-$(CONFIG_POSIX) += os-posix.o > > common-obj-$(CONFIG_LINUX) += fsdev/ > +common-obj-$(CONFIG_DARWIN) += fsdev/ > > common-obj-y += migration/ > > diff --git a/configure b/configure > index a8498ab..eb7328c 100755 > --- a/configure > +++ b/configure > @@ -5535,16 +5535,27 @@ if test "$want_tools" = "yes" ; then >fi > fi > if test "$softmmu" = yes ; then > - if test "$linux" = yes; then > -if test "$virtfs" != no && test "$cap" = yes && test "$attr" = yes ; then > + if test "$virtfs" != no; then > +if test "$linux" = yes; then > + if test "$cap" = yes && test "$attr" = yes ; then > +virtfs=yes > +tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)" > + else > +if test "$virtfs" = yes; then > + error_exit "VirtFS requires libcap devel and libattr devel under > Linux" > +fi > +virtfs=no > + fi > +elif test "$darwin" = yes; then >virtfs=yes > - tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)" So, no proxy helper on Darwin ? Why ? The limitation should be mentioned in the changelog at least. > else >if test "$virtfs" = yes; then > -error_exit "VirtFS requires libcap devel and libattr devel" > +error_exit "VirtFS is supported only on Linux and Darwin" >fi >virtfs=no > fi > + fi > + if test "$linux" = yes; then > if test "$mpath" != no && test "$mpathpersist" = yes ; then >mpath=yes > else > @@ -,10 +5566,6 @@ if test "$softmmu" = yes ; then > fi > tools="$tools scsi/qemu-pr-helper\$(EXESUF)" >else > -if test "$virtfs" = yes; then > - error_exit "VirtFS is supported only on Linux" > -fi > -virtfs=no > if test "$mpath" = yes; then >error_exit "Multipath is supported only on Linux" > fi
Re: [Qemu-devel] [PATCH v2 1/1] sandbox: disable -sandbox if CONFIG_SECCOMP undefined
在 2018/5/25 下午5:36, Eduardo Otubo 写道: On 05/25/2018 06:23 AM, Yi Min Zhao wrote: 在 2018/5/24 下午9:40, Paolo Bonzini 写道: On 24/05/2018 09:53, Eduardo Otubo wrote: Thanks! But I have not got response from Paolo. I have added him to CC list. I'll just wait one more ACK and will send a pull request on the seccomp queue. Thanks for the contribution. So... what I should do is wait? Yes, even though I think we're safe to proceed without his explicit ack. The patch is okay; however, as a follow-up, you could consider moving all the CONFIG_SECCOMP code to qemu-seccomp.c. This way, the only #ifdef remains the one around qemu_opts_foreach. Paolo Thanks for your comment! Indeed, moving to the single C file is much more clear. I will do this after this patch. @Otubo, what about next step? If you're willing to send v3 with the changes Paolo suggested, I can wait to send the pull request. No worries. OK. I will update the new version with Paolo's suggestion.
[Qemu-devel] [Bug 1773753] Re: virsh managed save fails with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc
with above patch compiled on top of latest upstream fails with below error: # virsh managedsave avocado-vt-vm1 error: Failed to save domain avocado-vt-vm1 state error: internal error: guest unexpectedly quit rest of the behaviour same.. # virsh start avocado-vt-vm1 gets hung ---crtl+c --> to comeback to prompt # # virsh destroy avocado-vt-vm1 Domain avocado-vt-vm1 destroyed # virsh undefine --managed-save avocado-vt-vm1 Domain avocado-vt-vm1 has been undefined -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1773753 Title: virsh managed save fails with qemu version v2.12.0-813-g5a5c383b13-dirty on powerpc Status in QEMU: New Bug description: Host Env: IBM Power8 with Fedora28 base with compiled upstream kernel, qemu, libvirt. Host Kernel: 4.17.0-rc5-00069-g3acf4e395260 qemu-kvm(5a5c383b1373aeb6c87a0d6060f6c3dc7c53082b): v2.12.0-813-g5a5c383b13-dirty libvirt(4804a4db33a37f828d033733bc47f6eff5d262c3): Guest Kernel: 4.17.0-rc7 Steps to recreate: Define a guest attached with above setup and start. # virsh start avocado-vt-vm1 guest console;... # uname -r 4.17.0-rc7 [root@atest-guest ~]# lscpu Architecture:ppc64le Byte Order: Little Endian CPU(s): 3 On-line CPU(s) list: 0-2 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s): 3 NUMA node(s):1 Model: 2.1 (pvr 004b 0201) Model name: POWER8 (architected), altivec supported Hypervisor vendor: KVM Virtualization type: para L1d cache: 64K L1i cache: 32K NUMA node0 CPU(s): 0-2 # virsh managedsave avocado-vt-vm1 Domain avocado-vt-vm1 state saved by libvirt # virsh list IdName State # virsh start avocado-vt-vm1 Hangs forever and vm state goes to paused. # virsh list IdName State 87avocado-vt-vm1 paused P:S:- with same above setup, just changing the qemu-kvm comes bydefault with F28 works fine. /usr/bin/qemu-kvm --version QEMU emulator version 2.11.1(qemu-2.11.1-2.fc28) Summary: with above other setup. machine type pseries-2.12 and qemu-2.11.1-2.fc28 -Works fine. machine type pseries-2.12/pseries-2.13 and qemu 5a5c383b1373aeb6c87a0d6060f6c3dc7c53082b - Does not work. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1773753/+subscriptions