Re: [PATCH v2] vhost-vsock: delete vqs in vhost_vsock_unrealize to avoid memleaks
On Wed, Jan 15, 2020 at 02:25:35PM +0800, pannengy...@huawei.com wrote:
> From: Pan Nengyuan
>
> Receive/transmit/event vqs forgot to cleanup in vhost_vsock_unrealize. This
> patch save receive/transmit vq pointer in realize() and cleanup vqs
> through those vq pointers in unrealize(). The leak stack is as follow:
>
> Direct leak of 21504 byte(s) in 3 object(s) allocated from:
> #0 0x7f86a1356970 (/lib64/libasan.so.5+0xef970) ??:?
> #1 0x7f86a09aa49d (/lib64/libglib-2.0.so.0+0x5249d) ??:?
> #2 0x5604852f85ca (./x86_64-softmmu/qemu-system-x86_64+0x2c3e5ca)
> /mnt/sdb/qemu/hw/virtio/virtio.c:2333
> #3 0x560485356208 (./x86_64-softmmu/qemu-system-x86_64+0x2c9c208)
> /mnt/sdb/qemu/hw/virtio/vhost-vsock.c:339
> #4 0x560485305a17 (./x86_64-softmmu/qemu-system-x86_64+0x2c4ba17)
> /mnt/sdb/qemu/hw/virtio/virtio.c:3531
> #5 0x5604858e6b65 (./x86_64-softmmu/qemu-system-x86_64+0x322cb65)
> /mnt/sdb/qemu/hw/core/qdev.c:865
> #6 0x5604861e6c41 (./x86_64-softmmu/qemu-system-x86_64+0x3b2cc41)
> /mnt/sdb/qemu/qom/object.c:2102
>
> Reported-by: Euler Robot
> Signed-off-by: Pan Nengyuan
> ---
> Changes V2 to V1:
> - delete virtqueues after vhost cleanup to avoid use-after-free
> - aslo delete virtqueues in the error path of realize()
> ---
> hw/virtio/vhost-vsock.c | 12 ++--
> include/hw/virtio/vhost-vsock.h | 2 ++
> 2 files changed, 12 insertions(+), 2 deletions(-)
Reviewed-by: Stefano Garzarella
>
> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
> index f5744363a8..b6cee479bb 100644
> --- a/hw/virtio/vhost-vsock.c
> +++ b/hw/virtio/vhost-vsock.c
> @@ -335,8 +335,10 @@ static void vhost_vsock_device_realize(DeviceState *dev,
> Error **errp)
> sizeof(struct virtio_vsock_config));
>
> /* Receive and transmit queues belong to vhost */
> -virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> vhost_vsock_handle_output);
> -virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> vhost_vsock_handle_output);
> +vsock->recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> + vhost_vsock_handle_output);
> +vsock->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> + vhost_vsock_handle_output);
>
> /* The event queue belongs to QEMU */
> vsock->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> @@ -363,6 +365,9 @@ static void vhost_vsock_device_realize(DeviceState *dev,
> Error **errp)
> err_vhost_dev:
> vhost_dev_cleanup(&vsock->vhost_dev);
> err_virtio:
> +virtio_delete_queue(vsock->recv_vq);
> +virtio_delete_queue(vsock->trans_vq);
> +virtio_delete_queue(vsock->event_vq);
> virtio_cleanup(vdev);
> close(vhostfd);
> return;
> @@ -379,6 +384,9 @@ static void vhost_vsock_device_unrealize(DeviceState
> *dev, Error **errp)
> vhost_vsock_set_status(vdev, 0);
>
> vhost_dev_cleanup(&vsock->vhost_dev);
> +virtio_delete_queue(vsock->recv_vq);
> +virtio_delete_queue(vsock->trans_vq);
> +virtio_delete_queue(vsock->event_vq);
> virtio_cleanup(vdev);
> }
>
> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
> index d509d67c4a..bc5a988ee5 100644
> --- a/include/hw/virtio/vhost-vsock.h
> +++ b/include/hw/virtio/vhost-vsock.h
> @@ -33,6 +33,8 @@ typedef struct {
> struct vhost_virtqueue vhost_vqs[2];
> struct vhost_dev vhost_dev;
> VirtQueue *event_vq;
> +VirtQueue *recv_vq;
> +VirtQueue *trans_vq;
> QEMUTimer *post_load_timer;
>
> /*< public >*/
> --
> 2.21.0.windows.1
>
>
>
--
Re: [PATCH V2] vhost-user-test: fix a memory leak
On 15/01/2020 04.10, Pan Nengyuan wrote: > > On 1/13/2020 10:32 AM, Pan Nengyuan wrote: >> >> On 1/12/2020 6:39 PM, Thomas Huth wrote: [...] >>> ... and now I had to unqueue the patch again. It is reproducibly causing >>> one of the gitlab CI pipelines to fail with a timeout, e.g.: >>> >>> https://gitlab.com/huth/qemu/-/jobs/400101552 >>> >>> Not sure what is going on here, though, there is no obvious error >>> message in the output... this needs some more investigation... do you >>> have a gitlab account and could have a look? >>> >> >> OK, I will register a account and have a look. >> > > I'm sorry, I build and test with the same params, but I can't reproduce it. > Could you add "V=1 or V=2" params to get more information ? It seems to hang forever in qos-test /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/announce-self : https://gitlab.com/huth/qemu/-/jobs/403472594 It's completely weird, I also added some fprintf statements: https://gitlab.com/huth/qemu/commit/8ae76c0cf37cf46d26620dd ... but none of them show up in the output of the test run... so I'm currently completely puzzled what might be going wrong here... Any other ideas what we could try here? Thomas
Re: Making QEMU easier for management tools and applications
Christophe de Dinechin writes:
> I started cutting some stuff out.
>
>> On 14 Jan 2020, at 14:04, Markus Armbruster wrote:
>>
>> Prior art:
>>
>>Presentation
>>KVM Forum 2017: Towards a More Expressive and Introspectable QEMU
>>Command Line
>>https://www.youtube.com/watch?v=gtpOLQgnwug
>>https://www.linux-kvm.org/images/f/f2/Armbru-qapi-cmdline_1.pdf
>>
>>RFC patches
>>https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg00209.html
>>Message-Id: <20171002152552.27999-1-arm...@redhat.com>
>>https://repo.or.cz/qemu/armbru.git/shortlog/refs/heads/qapi-cmdline
>
> Very useful, thanks.
>
>>
>>> Compatibility is much harder, which is not the least because the
>>> existing command line options are bad (primarily: using inconsistent
>>> syntax).
>>
>> Semantics are just as problematic.
>
> Yes, but often you can leverage something (e.g. JSON, XML, etc)
>
> We could decide that the new QAPI language is a custom implementation
> of Python ;-) Or more seriously, we could have some part of the JSON
> schema that are “escapes” to C code that we put in some generated file.
> I used {{ }} below for this escape, in which case the semantics would be
> that of C plus some specific macros we insert in it.
>
> { 'option': '--add-fd',
> 'data': { 'fd': 'int', 'set': 'int', '*opaque': 'str' },
> 'help': [
> "-add-fd fd=fd,set=set[,opaque=opaque]",
> "Add 'fd' to fd 'set'”]
> ‘validate’ : {{
> error_if(QOPT.fd < 0, “fd option cannot be negative”);
> error_if(QOPT.fd <= STDERR_FILENO, “fd cannot be a standard I/O stream”);
> }},
> ‘exec’ : {{
> if (do_add_fd(QOPT.fd, QOPT.fset, QOPT.opaque) < 0) {
> exit(1);
> }
> }},
>
> I’m not necessarily saying this is a good idea, mind you.
Precedence: the value of 'if' is also a C snippet.
>> You mean the qemu-options.hx. qemu-options.def is generated from it.
>
> Oh, I had missed that, thanks.
>
or build configuration (there is a hint of the connection between
the two in the option ‘if’ member in the .json files).
>>
>> I'm not sure what exactly you have in mind for build configuration.
>
> I meant what you pointed out wrt. options, that this “language” needs to
> know about the qemu configuration. There is ‘if’ today, you pointed
> out arch-specific for options. But the configuration itself is yet
> another “language” with its own semantics… which JSON knows
> nothing about.
QAPI 'if' is a pragmatic design balancing simplicty with expressiveness.
It's part of the QAPI language except for the actual conditions. The
QAPI generator blindly wraps whatever it generates for the thing
carrying an 'if': 'COND' in #if COND ... #endif /* COND */. In other
words, we express "what is conditional" in the QAPI language, but resort
to the C language for expressing the actual conditionals.
PRO:
* Supports arbitrary #if.
* QAPI language impact is light: just another (string-valued) property
of definitions and members.
* Generated code is the same regardless of QEMU configuration.
CON:
* Generating anything but C is problematic. Currently that's just
documentation. Having C conditionals there isn't exactly great, but
tolerable.
In other words, the language I have in mind would be something
that I could auto-generate from, say, the current qapi-schema.json
and the qemu-options.def. Maybe in the end, that would be simply
by having qemu-options.def being used to build qemu-options.json,
and then add the relevant entries in qemu-options.json. Maybe
there is a better way.
>>>
>>> I would rather say that .def should go away and its content should
>>> be converted to a JSON schema that becomes the new source format rather
>>> than an intermediate generated file. There is nothing in .def files that
>>> couldn't be represented in the schema.
>>
>> Yes.
>
> Agreed too. The meta-schema would need to be extended.
>
>>>
>>> To a certain degree, QAPI does just that, by supporting different data
>>> types for options. If necessary, I'm sure the type system could be
>>> extended, but it's not clear to me to which degree we actually need
>>> this.
>>
>> QAPI is much more expressive than qemu-options.hx. All the latter can
>> do is "has option argument" and "option is arch-specific". QAPI lets
>> you specify an option argument's structure, and supports compile-time
>> conditionals. It can't do things like "must specify either argument A
>> or B", or "numeric argument C must not exceed argument D", or "multiple
>> options combine vs. last one wins". Mostly because "it’s already
>> complicated enough as is".
>
> Understood.
>
>
>>>
>>> We could. But is it actually worth inventing a new programming language?
>>> I think this is something that should be done in C code even in the
>>> future. I think what we're looking for is a way to describe interfaces,
>>> not implementations.
>>
>> Sane sugar can be described declaratively as macro expansion.
>>
>> Less than s
Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
在 2020/1/15 14:30, Michael S. Tsirkin 写道: Problem is IASL disassembler still doesn't work on all hosts we want to support. And its output isn't really stable enough to act as a golden master. Until we have a better tool, I propose the contributor just follows all steps 1-6. The reason they have been listed as maintainer action items is really just so that multiple patches affecting same ACPI table can be applied, with maintainer resolving conflicts himself. But this job can be pushed to contributors if as in the case of ARM maintainer isn't really interested in reading ACPI code anyway. So I propose the following patch - comments? Signed-off-by: Michael S. Tsirkin diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index f1ac2d7e96..3a6a3e7257 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -16,7 +16,10 @@ * 1. add empty files for new tables, if any, under tests/data/acpi * 2. list any changed files in tests/bios-tables-test-allowed-diff.h * 3. commit the above *before* making changes that affect the tables - * Maintainer: + * + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve conflicts + * in binary commit created in step 6): + * * After 1-3 above tests will pass but ignore differences with the expected files. * You will also notice that tests/bios-tables-test-allowed-diff.h lists * a bunch of files. This is your hint that you need to do the below: @@ -28,13 +31,17 @@ * output. If not - disassemble them yourself in any way you like. * Look at the differences - make sure they make sense and match what the * changes you are merging are supposed to do. + * Save the changes, preferably in form of ASL diff for the the commit log in NIT: 2 "the" before commit log + * step 6. * * 5. From build directory, run: * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh - * 6. Now commit any changes. - * 7. Before doing a pull request, make sure tests/bios-tables-test-allowed-diff.h - *is empty - this will ensure following changes to ACPI tables will - *be noticed. + * 6. Now commit any changes to the expected binary, include diff from step 4 + *in commit log. + * 7. Before sending patches to the list (Contributor) + *or before doing a pull request (Maintainer), make sure + *tests/bios-tables-test-allowed-diff.h is empty - this will ensure + *following changes to ACPI tables will be noticed. */ For contributors doing the full work, does that mean the patchset sent to the list contains the following parts? 1. patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. 2. patches 2-n: real changes, may contain multiple patches. 3. patch n+1: update golden master binaries and empty tests/bios-tables-test-allowed-diff.h Thanks, Heyi #include "qemu/osdep.h" .
Re: Making QEMU easier for management tools and applications
Thanks a bunch. This clarifies a number of my misconceptions about how this is currently used. Most notably this one: > On 15 Jan 2020, at 10:20, Markus Armbruster wrote: > >> We don’t want the QAPI to let arbitrary fields of a QOM object >> be modified, do we? > > We already do: QMP command qom-set. If it breaks your guest, you get to > keep the pieces. Ouch. I certainly did not expect that. "It is not what you don’t know that kills you, it is what you know that ain’t so". > >>> http://dreamsongs.com/RiseOfWorseIsBetter.html >> >> You know that I positively hate this ;-) > > It's been a tough lesson for me, too. Not sure I can call it a “lesson”. Much like a philosophy to fight against, IMO. >> Well, I guess we can expand the schema. #ILoveJSON. > > Basing the QAPI language on JSON was a poor choice. Not sure that's > fixable at a reasonable cost. Well, at least now I have a slightly better understanding of the related costs and trade-offs. Thanks a lot for explaining. Christophe
Re: Making QEMU easier for management tools and applications
Hi On Wed, Jan 15, 2020 at 1:21 PM Markus Armbruster wrote: > > Christophe de Dinechin writes: > > >> To make this worthwhile, we'd have to replace dynamic QOM properties by > >> static ones when possible. Monumental task. > > > > I’m sure you are right, but it’s hard for me to evaluate, given how > > many ways there are to access an object. Naively, grepping for > > set_prop and for new_with_prop does not give me that many hits. > > Look for object_property_add*(). Some 450 hits. fwiw, I have started tackling that. Easy first step is to move all QDev properties to class properties, and this is done in : https://patchew.org/QEMU/20200110153039.1379601-1-marcandre.lur...@redhat.com/ Moving from instance to class properties is straightforward many times (when the property is unconditonally added in instance init for example). There are a few complicated cases though. To me, the most problematic is reviewer-time and willingness to do such low-benefits changes. > Basing the QAPI language on JSON was a poor choice. Not sure that's > fixable at a reasonable cost. Translating it to another language should be relatively easy, but to what? -- Marc-André Lureau
Re: [PATCH 1/3] target/i386: Renumber EXCP_SYSCALL
Richard Henderson writes: > We are not short of numbers for EXCP_*. There is no need to confuse things > by having EXCP_VMEXIT and EXCP_SYSCALL overlap, even though the former is > only used for system mode and the latter is only used for user mode. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée > --- > target/i386/cpu.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 594326a794..164d038d1f 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -998,9 +998,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; > #define EXCP11_ALGN 17 > #define EXCP12_MCHK 18 > > -#define EXCP_SYSCALL0x100 /* only happens in user only emulation > - for syscall instruction */ > -#define EXCP_VMEXIT 0x100 > +#define EXCP_VMEXIT 0x100 /* only for system emulation */ > +#define EXCP_SYSCALL0x101 /* only for user emulation */ > > /* i386-specific interrupt pending bits. */ > #define CPU_INTERRUPT_POLL CPU_INTERRUPT_TGT_EXT_1 -- Alex Bennée
Re: [PATCH 2/3] linux-user/i386: Split out gen_signal
Richard Henderson writes:
> This is a bit tidier than open-coding the 5 lines necessary
> to initialize the target_siginfo_t. In addition, this zeros
> the remaining bytes of the target_siginfo_t, rather than
> passing in garbage.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> linux-user/i386/cpu_loop.c | 93 ++
> 1 file changed, 33 insertions(+), 60 deletions(-)
>
> diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
> index 024b6f4d58..e217cca5ee 100644
> --- a/linux-user/i386/cpu_loop.c
> +++ b/linux-user/i386/cpu_loop.c
> @@ -81,13 +81,23 @@ static void set_idt(int n, unsigned int dpl)
> }
> #endif
>
> +static void gen_signal(CPUX86State *env, int sig, int code, abi_ptr addr)
> +{
> +target_siginfo_t info = {
> +.si_signo = sig,
> +.si_code = code,
> +._sifields._sigfault._addr = addr
> +};
> +
> +queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +}
> +
> void cpu_loop(CPUX86State *env)
> {
> CPUState *cs = env_cpu(env);
> int trapnr;
> abi_ulong pc;
> abi_ulong ret;
> -target_siginfo_t info;
>
> for(;;) {
> cpu_exec_start(cs);
> @@ -134,70 +144,45 @@ void cpu_loop(CPUX86State *env)
> #endif
> case EXCP0B_NOSEG:
> case EXCP0C_STACK:
> -info.si_signo = TARGET_SIGBUS;
> -info.si_errno = 0;
> -info.si_code = TARGET_SI_KERNEL;
> -info._sifields._sigfault._addr = 0;
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +gen_signal(env, TARGET_SIGBUS, TARGET_SI_KERNEL, 0);
> break;
> case EXCP0D_GPF:
> /* XXX: potential problem if ABI32 */
> #ifndef TARGET_X86_64
> if (env->eflags & VM_MASK) {
> handle_vm86_fault(env);
> -} else
> -#endif
> -{
> -info.si_signo = TARGET_SIGSEGV;
> -info.si_errno = 0;
> -info.si_code = TARGET_SI_KERNEL;
> -info._sifields._sigfault._addr = 0;
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +break;
> }
> +#endif
> +gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0);
> break;
> case EXCP0E_PAGE:
> -info.si_signo = TARGET_SIGSEGV;
> -info.si_errno = 0;
> -if (!(env->error_code & 1))
> -info.si_code = TARGET_SEGV_MAPERR;
> -else
> -info.si_code = TARGET_SEGV_ACCERR;
> -info._sifields._sigfault._addr = env->cr[2];
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +gen_signal(env, TARGET_SIGSEGV,
> + (env->error_code & 1 ?
> +TARGET_SEGV_ACCERR : TARGET_SEGV_MAPERR),
> + env->cr[2]);
> break;
> case EXCP00_DIVZ:
> #ifndef TARGET_X86_64
> if (env->eflags & VM_MASK) {
> handle_vm86_trap(env, trapnr);
> -} else
> -#endif
> -{
> -/* division by zero */
> -info.si_signo = TARGET_SIGFPE;
> -info.si_errno = 0;
> -info.si_code = TARGET_FPE_INTDIV;
> -info._sifields._sigfault._addr = env->eip;
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +break;
> }
> +#endif
> +gen_signal(env, TARGET_SIGFPE, TARGET_FPE_INTDIV, env->eip);
> break;
> case EXCP01_DB:
> case EXCP03_INT3:
> #ifndef TARGET_X86_64
> if (env->eflags & VM_MASK) {
> handle_vm86_trap(env, trapnr);
> -} else
> +break;
> +}
> #endif
> -{
> -info.si_signo = TARGET_SIGTRAP;
> -info.si_errno = 0;
> -if (trapnr == EXCP01_DB) {
> -info.si_code = TARGET_TRAP_BRKPT;
> -info._sifields._sigfault._addr = env->eip;
> -} else {
> -info.si_code = TARGET_SI_KERNEL;
> -info._sifields._sigfault._addr = 0;
> -}
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> +if (trapnr == EXCP01_DB) {
> +gen_signal(env, TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->eip);
> +} else {
> +gen_signal(env, TARGET_SIGTRAP, TARGET_SI_KERNEL, 0);
> }
> break;
> case EXCP04_INTO:
> @@ -205,31 +190,19 @@ void cpu_loop(CPUX86State *env)
> #ifndef TARGET_X86_64
> if (env->eflags & VM_MASK) {
> handle_vm86_trap(env, trapnr);
> -} else
> -#endif
> -{
> -info.si_signo = TARGET_SIGSEGV;
> -
Re: [PATCH 0/3] linux-user: Implement x86_64 vsyscalls
Le 14/01/2020 à 22:09, Richard Henderson a écrit : > The x86_64 abi has a legacy vsyscall page. The kernel folk > have been trying to deprecate this since at least v3.1, but > > (1) We don't implement the vdso that replaces vsyscalls, > (2) As of v5.5, the vsyscall page is still enabled by default. > > This lack is affecting Peter's linux-user testing. > > The dependency is not obvious because Peter is running the tests > on x86_64, so the host is providing a vsyscall page to qemu. > > Because of how user-only memory operations are handled, with no > validation of guest vs host pages, so long as qemu chooses to > run with guest_base == 0, the guest may Just So Happen to read > the host's vsyscall page. > > Complicating this, new OS releases may use a kernel configured > with CONFIG_LEGACY_VSYSCALL_XONLY=y, which means the the vsyscall > page cannot be read, only executed. Which means that the guest > then cannot read the host vsyscall page during translation and > will SIGSEGV. > > Exactly which of these many variables is affecting Peter's testing > with Ubuntu 18.04 of my TCG merge, I'm not exactly sure. I suspect > that it is the change to drop the textseg_addr adjustment to user-only > static binaries. IIRC bionic does not support -static-pie, which is > the preferred replacement. This could mean that the host and guest > binaries overlap, which leads to guest_base != 0. > > I vaguely remember someone (Paolo?) implementing something like > this many years ago, but clearly it never got merged. > > In any case, this emulation has been missing for too long. > > > r~ > > > Richard Henderson (3): > target/i386: Renumber EXCP_SYSCALL > linux-user/i386: Split out gen_signal > linux-user/i386: Emulate x86_64 vsyscalls > > target/i386/cpu.h | 6 +- > linux-user/i386/cpu_loop.c | 197 ++--- > target/i386/translate.c| 16 ++- > 3 files changed, 155 insertions(+), 64 deletions(-) > Thank you Richard. I'll take this series through the linux-user branch except if you prefer to do the pull request yourself via another branch (x86 tcg?). Thanks, Laurent
Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
On Wed, Jan 15, 2020 at 05:25:50PM +0800, Guoheyi wrote: > > 在 2020/1/15 14:30, Michael S. Tsirkin 写道: > > Problem is IASL disassembler still doesn't work on all hosts > > we want to support. And its output isn't really stable enough > > to act as a golden master. > > > > Until we have a better tool, I propose the contributor just follows all > > steps 1-6. The reason they have been listed as maintainer action items > > is really just so that multiple patches affecting same ACPI table > > can be applied, with maintainer resolving conflicts himself. > > But this job can be pushed to contributors if as in the case of ARM > > maintainer isn't really interested in reading ACPI code anyway. > > > > So I propose the following patch - comments? > > > > Signed-off-by: Michael S. Tsirkin > > > > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > > index f1ac2d7e96..3a6a3e7257 100644 > > --- a/tests/qtest/bios-tables-test.c > > +++ b/tests/qtest/bios-tables-test.c > > @@ -16,7 +16,10 @@ > >* 1. add empty files for new tables, if any, under tests/data/acpi > >* 2. list any changed files in tests/bios-tables-test-allowed-diff.h > >* 3. commit the above *before* making changes that affect the tables > > - * Maintainer: > > + * > > + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve > > conflicts > > + * in binary commit created in step 6): > > + * > >* After 1-3 above tests will pass but ignore differences with the > > expected files. > >* You will also notice that tests/bios-tables-test-allowed-diff.h lists > >* a bunch of files. This is your hint that you need to do the below: > > @@ -28,13 +31,17 @@ > >* output. If not - disassemble them yourself in any way you like. > >* Look at the differences - make sure they make sense and match what the > >* changes you are merging are supposed to do. > > + * Save the changes, preferably in form of ASL diff for the the commit log > > in > NIT: 2 "the" before commit log > > + * step 6. > >* > >* 5. From build directory, run: > >* $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh > > - * 6. Now commit any changes. > > - * 7. Before doing a pull request, make sure > > tests/bios-tables-test-allowed-diff.h > > - *is empty - this will ensure following changes to ACPI tables will > > - *be noticed. > > + * 6. Now commit any changes to the expected binary, include diff from > > step 4 > > + *in commit log. > > + * 7. Before sending patches to the list (Contributor) > > + *or before doing a pull request (Maintainer), make sure > > + *tests/bios-tables-test-allowed-diff.h is empty - this will ensure > > + *following changes to ACPI tables will be noticed. > >*/ > > For contributors doing the full work, does that mean the patchset sent to > the list contains the following parts? > > 1. patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. > > 2. patches 2-n: real changes, may contain multiple patches. > > 3. patch n+1: update golden master binaries and empty > tests/bios-tables-test-allowed-diff.h > > Thanks, > > Heyi Yes. > > #include "qemu/osdep.h" > > > > > > .
Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
On Wed, Jan 15, 2020 at 05:25:50PM +0800, Guoheyi wrote: > > 在 2020/1/15 14:30, Michael S. Tsirkin 写道: > > Problem is IASL disassembler still doesn't work on all hosts > > we want to support. And its output isn't really stable enough > > to act as a golden master. > > > > Until we have a better tool, I propose the contributor just follows all > > steps 1-6. The reason they have been listed as maintainer action items > > is really just so that multiple patches affecting same ACPI table > > can be applied, with maintainer resolving conflicts himself. > > But this job can be pushed to contributors if as in the case of ARM > > maintainer isn't really interested in reading ACPI code anyway. > > > > So I propose the following patch - comments? > > > > Signed-off-by: Michael S. Tsirkin > > > > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > > index f1ac2d7e96..3a6a3e7257 100644 > > --- a/tests/qtest/bios-tables-test.c > > +++ b/tests/qtest/bios-tables-test.c > > @@ -16,7 +16,10 @@ > >* 1. add empty files for new tables, if any, under tests/data/acpi > >* 2. list any changed files in tests/bios-tables-test-allowed-diff.h > >* 3. commit the above *before* making changes that affect the tables > > - * Maintainer: > > + * > > + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve > > conflicts > > + * in binary commit created in step 6): > > + * > >* After 1-3 above tests will pass but ignore differences with the > > expected files. > >* You will also notice that tests/bios-tables-test-allowed-diff.h lists > >* a bunch of files. This is your hint that you need to do the below: > > @@ -28,13 +31,17 @@ > >* output. If not - disassemble them yourself in any way you like. > >* Look at the differences - make sure they make sense and match what the > >* changes you are merging are supposed to do. > > + * Save the changes, preferably in form of ASL diff for the the commit log > > in > NIT: 2 "the" before commit log > > + * step 6. > >* > >* 5. From build directory, run: > >* $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh > > - * 6. Now commit any changes. > > - * 7. Before doing a pull request, make sure > > tests/bios-tables-test-allowed-diff.h > > - *is empty - this will ensure following changes to ACPI tables will > > - *be noticed. > > + * 6. Now commit any changes to the expected binary, include diff from > > step 4 > > + *in commit log. > > + * 7. Before sending patches to the list (Contributor) > > + *or before doing a pull request (Maintainer), make sure > > + *tests/bios-tables-test-allowed-diff.h is empty - this will ensure > > + *following changes to ACPI tables will be noticed. > >*/ > > For contributors doing the full work, does that mean the patchset sent to > the list contains the following parts? > > 1. patch 1: list changed files in tests/bios-tables-test-allowed-diff.h. > > 2. patches 2-n: real changes, may contain multiple patches. > > 3. patch n+1: update golden master binaries and empty > tests/bios-tables-test-allowed-diff.h > > Thanks, > > Heyi Here's a hopefully better patch. Peter does this address the issue? Signed-off-by: Michael S. Tsirkin diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index f1ac2d7e96..3ab4872bd7 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -16,7 +16,10 @@ * 1. add empty files for new tables, if any, under tests/data/acpi * 2. list any changed files in tests/bios-tables-test-allowed-diff.h * 3. commit the above *before* making changes that affect the tables - * Maintainer: + * + * Contributor or ACPI Maintainer (steps 4-7 need to be redone to resolve conflicts + * in binary commit created in step 6): + * * After 1-3 above tests will pass but ignore differences with the expected files. * You will also notice that tests/bios-tables-test-allowed-diff.h lists * a bunch of files. This is your hint that you need to do the below: @@ -28,13 +31,23 @@ * output. If not - disassemble them yourself in any way you like. * Look at the differences - make sure they make sense and match what the * changes you are merging are supposed to do. + * Save the changes, preferably in form of ASL diff for the commit log in + * step 6. * * 5. From build directory, run: * $(SRC_PATH)/tests/data/acpi/rebuild-expected-aml.sh - * 6. Now commit any changes. - * 7. Before doing a pull request, make sure tests/bios-tables-test-allowed-diff.h - *is empty - this will ensure following changes to ACPI tables will - *be noticed. + * 6. Now commit any changes to the expected binary, include diff from step 4 + *in commit log. + * 7. Before sending patches to the list (Contributor) + *or before doing a pull request (Maintainer), make sure + *tests/bios-tables-test-allowed-diff.h is empty - this will ensure + *following changes to ACPI
Re: Feedback on multi-process QEMU muser prototype
Hi, > On Jan 14, 2020, at 3:22 PM, Stefan Hajnoczi wrote: > > I haven't seen the link to the muser prototype shared on the list yet, > so I'm taking the liberty of posting it for discussion: > https://github.com/oracle/qemu/tree/multi-process-qemu-v0.4.1-muser > > Great that a lot of the multi-process patch series is no longer > necessary. The muser approach requires less code in QEMU. > > The following points came to mind: > > 1. Configure PCI configuration space, BARs, and MSI/IRQs based on the > PCIDevice > instead of hard-coding the LSI SCSI controller's specifics. That way any > PCIDevice can run as an muser device. > > 2. Integrate with QEMU's event loop instead of spawning threads and calling > lm_ctx_run(). The event loop should monitor the muser fd for activity using > aio_set_fd_handler() and then call into libmuser to handle the event. This > will avoid thread model problems in the future and also allow true > multi-threading (IOThreads). Allowing muser to be used like that is in our to-do list. (+ Thanos / Swapnil). We have to extend muser.ko to allow the device file descriptor to be "pollable". Let me know how soon you want to see that so we can prioritise accordingly or assist someone in doing the work. F. > > 3. Drop previous multi-process QEMU commits if they are not needed. > > Stefan
RE: [PATCH] tests: acpi: update path in rebuild-expected-aml
> -Original Message-
> From: Thomas Huth [mailto:th...@redhat.com]
> Sent: 14 January 2020 17:08
> To: Shameerali Kolothum Thodi ;
> qemu-devel@nongnu.org; imamm...@redhat.com; m...@redhat.com
> Cc: xuwei (O) ; Linuxarm
> Subject: Re: [PATCH] tests: acpi: update path in rebuild-expected-aml
>
> On 14/01/2020 17.51, Shameer Kolothum wrote:
> > Since commit 1e8a1fae7464("test: Move qtests to a separate
> > directory") qtests are now placed in a separate folder and
> > this breaks the script used to rebuild the expected ACPI
> > tables for bios-tables-test. Update the script with correct
> > path.
> >
> > Fixes: 1e8a1fae7464("test: Move qtests to a separate directory")
> > Signed-off-by: Shameer Kolothum
> > ---
> > tests/data/acpi/rebuild-expected-aml.sh | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/data/acpi/rebuild-expected-aml.sh
> b/tests/data/acpi/rebuild-expected-aml.sh
> > index f89d4624bc..d44e511533 100755
> > --- a/tests/data/acpi/rebuild-expected-aml.sh
> > +++ b/tests/data/acpi/rebuild-expected-aml.sh
> > @@ -14,7 +14,7 @@
> >
> > qemu_bins="x86_64-softmmu/qemu-system-x86_64
> aarch64-softmmu/qemu-system-aarch64"
> >
> > -if [ ! -e "tests/bios-tables-test" ]; then
> > +if [ ! -e "tests/qtest/bios-tables-test" ]; then
> > echo "Test: bios-tables-test is required! Run make check before this
> script."
> > echo "Run this script from the build directory."
> > exit 1;
> > @@ -26,11 +26,11 @@ for qemu in $qemu_bins; do
> > echo "Also, run this script from the build directory."
> > exit 1;
> > fi
> > -TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu
> tests/bios-tables-test
> > +TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu
> tests/qtest/bios-tables-test
> > done
> >
> > eval `grep SRC_PATH= config-host.mak`
> >
> > -echo '/* List of comma-separated changed AML files to ignore */' >
> ${SRC_PATH}/tests/bios-tables-test-allowed-diff.h
> > +echo '/* List of comma-separated changed AML files to ignore */' >
> ${SRC_PATH}/tests/qtest/bios-tables-test-allowed-diff.h
> >
> > echo "The files were rebuilt and can be added to git."
>
> Oh, sorry for missing that in my patch series ... is there maybe a way
> that we could test this script in one of our CI pipelines so that it is
> not so easy to miss?
Right. That will be a useful one.
I am also seeing another error when I run "make check-qtest" on x86_64.
This doesn’t seems to be related to the recent changes. I have gone back
to 4.1.0 and it is still there.
TESTcheck-qtest-x86_64: tests/boot-order-test
TESTcheck-qtest-x86_64: tests/bios-tables-test
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
acpi-test: Warning! FACP binary file mismatch. Actual [aml:/tmp/aml-2Q9EE0],
Expected [aml:tests/data/acpi/pc/FACP.bridge].
acpi-test: Warning! FACP mismatch. Actual [asl:/tmp/asl-CQ9EE0.dsl,
aml:/tmp/aml-2Q9EE0], Expected [asl:/tmp/asl-N18EE0.dsl,
aml:tests/data/acpi/pc/FACP.bridge].
**
ERROR:tests/bios-tables-test.c:447:test_acpi_asl: assertion failed:
(all_tables_match)
ERROR - Bail out! ERROR:tests/bios-tables-test.c:447:test_acpi_asl: assertion
failed: (all_tables_match)
Aborted (core dumped)
/home/shameer/qemu/tests/Makefile.include:899: recipe for target
'check-qtest-x86_64' failed
make: *** [check-qtest-x86_64] Error 1
FACP seems to have changed and it looks like need to run the script to generate
a new one.
~/qemu$ diff -u /tmp/asl-CQ9EE0.dsl /tmp/asl-N18EE0.dsl
--- /tmp/asl-CQ9EE0.dsl 2020-01-15 10:52:03.018448627 +
+++ /tmp/asl-N18EE0.dsl 2020-01-15 10:52:03.022448627 +
@@ -3,7 +3,7 @@
* AML/ASL+ Disassembler version 20180105 (64-bit version)
* Copyright (c) 2000 - 2018 Intel Corporation
*
- * Disassembly of /tmp/aml-2Q9EE0, Wed Jan 15 10:52:03 2020
+ * Disassembly of tests/data/acpi/pc/FACP.bridge, Wed Jan 15 10:52:03
+ 2020
*
* ACPI Data Table [FACP]
*
@@ -13,7 +13,7 @@
[000h 4]Signature : "FACP"[Fixed ACPI
Description Table (FADT)]
[004h 0004 4] Table Length : 0074
[008h 0008 1] Revision : 01
-[009h 0009 1] Checksum : A1
+[009h 0009 1] Checksum : D3
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPCFACP"
[018h 0024 4] Oem Revision : 0001
@@ -54,7 +54,7 @@
[069h 0105 1] Duty Cycle Width : 00
[06Ah 0106 1]
[PATCH] m68k: Fix regression causing Single-Step via GDB/RSP to not single step
A regression that was introduced, with the refactor to TranslatorOps,
drops two lines that update the PC when single-stepping is being performed.
Fixes: 11ab74b01e0a ("target/m68k: Convert to TranslatorOps")
Reported-by: Lucien Murray-Pitts
Suggested-by: Lucien Murray-Pitts
Suggested-by: Richard Henderson
Signed-off-by: Laurent Vivier
---
Notes:
v2: update patch from Lucien with changes from Richard
update subject to prefix it with "m68k:"
rebase
target/m68k/translate.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index fcdb7bc8e4..a400c2295f 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6198,29 +6198,36 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase,
CPUState *cpu)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
-if (dc->base.is_jmp == DISAS_NORETURN) {
-return;
-}
-if (dc->base.singlestep_enabled) {
-gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
-return;
-}
-
switch (dc->base.is_jmp) {
+case DISAS_NORETURN:
+break;
case DISAS_TOO_MANY:
update_cc_op(dc);
-gen_jmp_tb(dc, 0, dc->pc);
+if (dc->base.singlestep_enabled) {
+tcg_gen_movi_i32(QREG_PC, dc->pc);
+gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
+} else {
+gen_jmp_tb(dc, 0, dc->pc);
+}
break;
case DISAS_JUMP:
/* We updated CC_OP and PC in gen_jmp/gen_jmp_im. */
-tcg_gen_lookup_and_goto_ptr();
+if (dc->base.singlestep_enabled) {
+gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
break;
case DISAS_EXIT:
/*
* We updated CC_OP and PC in gen_exit_tb, but also modified
* other state that may require returning to the main loop.
*/
-tcg_gen_exit_tb(NULL, 0);
+if (dc->base.singlestep_enabled) {
+gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
+} else {
+tcg_gen_exit_tb(NULL, 0);
+}
break;
default:
g_assert_not_reached();
--
2.24.1
Re: [qemu-web PATCH] documentation: update links to readthedocs
On Tue, Jan 14, 2020 at 12:11:34PM +0100, Thomas Huth wrote: > On 13/01/2020 11.35, Alex Bennée wrote: > > ..and extemporise a little about their state. > > > > Signed-off-by: Alex Bennée > > --- > > documentation.md | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/documentation.md b/documentation.md > > index f4ef9f4..55d5db6 100644 > > --- a/documentation.md > > +++ b/documentation.md > > @@ -3,9 +3,12 @@ title: QEMU documentation > > permalink: /documentation/ > > --- > > > > -The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be read > > online, courtesy of Stefan Weil. > > -More documentation is found in the > href="https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master";>`docs` > > -directory of the QEMU git tree. > > +The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be > > +read online, courtesy of Stefan Weil. There is a partial set of > > +[developer documentation](https://qemu.readthedocs.io/en/latest/) > > +which is generated from the QEMU git tree. The process of converting > > +the rest of the > > [`docs`](https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master) > > +directory is ongoing. > > This has a conflict with Stefan's patch to point to our documentation on > www.qemu.org now instead: > > https://patchwork.kernel.org/patch/11234545/ > > ... Stefan, looks like the index.html page is still not there yet, > although your other patch that includes index.html.in in the sources is > in the repository now? What's the status here? The qemu.git/master docs are built nightly here (index.html is now visible!): https://www.qemu.org/docs/master/ qemu.org's docs are more useful at the moment since they include the user documentation in addition to the developer documentation. It's good that we got ownership of the readthedocs.org account for QEMU. I don't know if rtd will be capable of building QEMU's hodgepodge of different documentation systems. It supports Sphinx and Mkdocs but that's not enough. Can we redirect readthedocs to qemu.org? Stefan signature.asc Description: PGP signature
Re: [PATCH 084/104] Virtiofsd: fix memory leak on fuse queueinfo
> From: Liu Bo
>
> For fuse's queueinfo, both queueinfo array and queueinfos are allocated in
> fv_queue_set_started() but not cleaned up when the daemon process quits.
>
> This fixes the leak in proper places.
>
> Signed-off-by: Liu Bo
> Signed-off-by: Eric Ren
> ---
> tools/virtiofsd/fuse_virtio.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index 7b22ae8d4f..a364f23d5d 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -671,6 +671,8 @@ static void fv_queue_set_started(VuDev *dev, int qidx,
> bool started)
> }
> close(ourqi->kill_fd);
> ourqi->kick_fd = -1;
> +free(vud->qi[qidx]);
> +vud->qi[qidx] = NULL;
> }
> }
>
> @@ -878,6 +880,13 @@ int virtio_session_mount(struct fuse_session *se)
> void virtio_session_close(struct fuse_session *se)
> {
> close(se->vu_socketfd);
I beleve above close() should be removed as it is called 6 line below.
> +
> +if (!se->virtio_dev) {
> +return;
> +}
> +
> +close(se->vu_socketfd);
> +free(se->virtio_dev->qi);
> free(se->virtio_dev);
> se->virtio_dev = NULL;
> }
> --
> 2.23.0
[PATCH] hw/input: Do not enable CONFIG_PCKBD by default
The i8042 device is part of the chipset of a machine, so it should be selected by the machines or chipsets (e.g. SuperIO chipsets), and not be enabled by default. Signed-off-by: Thomas Huth --- hw/input/Kconfig | 1 - hw/isa/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/input/Kconfig b/hw/input/Kconfig index 287f08887b..a2f25725be 100644 --- a/hw/input/Kconfig +++ b/hw/input/Kconfig @@ -7,7 +7,6 @@ config LM832X config PCKBD bool -default y select PS2 depends on ISA_BUS diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index 8a38813cc1..c7f07854f7 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -16,6 +16,7 @@ config I82378 config ISA_SUPERIO bool select ISA_BUS +select PCKBD config PC87312 bool -- 2.18.1
Re: [PATCH] virtio-9p-device: fix memleak in virtio_9p_device_unrealize
On Mittwoch, 15. Januar 2020 02:28:03 CET Pan Nengyuan wrote: > >>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > >>> index b5a7c03f26..b146387ae2 100644 > >>> --- a/hw/9pfs/virtio-9p-device.c > >>> +++ b/hw/9pfs/virtio-9p-device.c > >>> @@ -215,6 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState > >>> *dev, Error **errp) V9fsVirtioState *v = VIRTIO_9P(dev); > >>> > >>> V9fsState *s = &v->state; > >>> > >>> +virtio_delete_queue(v->vq); > >>> > >>> virtio_cleanup(vdev); > >>> v9fs_device_unrealize_common(s, errp); > >>> > >>> } > >> > >> Looks like you are using an old interface. The new one is > >> > >>void virtio_del_queue(VirtIODevice *vdev, int n); > > > > Ah, my bad, it is actually the other way around, that is I was not up to > > date; virtio_delete_queue() was apparently introduced a week ago: > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg00723.html > > > > The old virtio_del_queue() still exists though: > > https://github.com/qemu/qemu/blob/master/hw/virtio/virtio.c#L2339 > > > > Having said that, it would probably still make sense to use > > virtio_del_queue() instead for now to make it easier for stable branches > > to merge this fix? > virtio_delete_queue makes the cleanup more clear and it tends to replace the > old one gradually. So I think it would probably still use > virtio_delete_queue. > > Given that, maybe we can split this patch? > For example: > 1. use virtio_del_queue to fix memleak, it's easier for stable to merge. > 2. rename virtio_del_queue to virtio_delete_queue Exactly what I had in mind. :) Thanks! Best regards, Christian Schoenebeck
Re: [PATCH] hw/input: Do not enable CONFIG_PCKBD by default
Le 15/01/2020 à 12:37, Thomas Huth a écrit : > The i8042 device is part of the chipset of a machine, so it should > be selected by the machines or chipsets (e.g. SuperIO chipsets), > and not be enabled by default. > > Signed-off-by: Thomas Huth > --- > hw/input/Kconfig | 1 - > hw/isa/Kconfig | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/input/Kconfig b/hw/input/Kconfig > index 287f08887b..a2f25725be 100644 > --- a/hw/input/Kconfig > +++ b/hw/input/Kconfig > @@ -7,7 +7,6 @@ config LM832X > > config PCKBD > bool > -default y > select PS2 > depends on ISA_BUS > > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > index 8a38813cc1..c7f07854f7 100644 > --- a/hw/isa/Kconfig > +++ b/hw/isa/Kconfig > @@ -16,6 +16,7 @@ config I82378 > config ISA_SUPERIO > bool > select ISA_BUS > +select PCKBD > > config PC87312 > bool > It is also used by R4K but the Kconfig already includes it, so: Reviewed-by: Laurent Vivier
Re: [qemu-web PATCH] documentation: update links to readthedocs
Stefan Hajnoczi writes: > On Tue, Jan 14, 2020 at 12:11:34PM +0100, Thomas Huth wrote: >> On 13/01/2020 11.35, Alex Bennée wrote: >> > ..and extemporise a little about their state. >> > >> > Signed-off-by: Alex Bennée >> > --- >> > documentation.md | 9 ++--- >> > 1 file changed, 6 insertions(+), 3 deletions(-) >> > >> > diff --git a/documentation.md b/documentation.md >> > index f4ef9f4..55d5db6 100644 >> > --- a/documentation.md >> > +++ b/documentation.md >> > @@ -3,9 +3,12 @@ title: QEMU documentation >> > permalink: /documentation/ >> > --- >> > >> > -The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be >> > read online, courtesy of Stefan Weil. >> > -More documentation is found in the > > href="https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master";>`docs` >> > -directory of the QEMU git tree. >> > +The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be >> > +read online, courtesy of Stefan Weil. There is a partial set of >> > +[developer documentation](https://qemu.readthedocs.io/en/latest/) >> > +which is generated from the QEMU git tree. The process of converting >> > +the rest of the >> > [`docs`](https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master) >> > +directory is ongoing. >> >> This has a conflict with Stefan's patch to point to our documentation on >> www.qemu.org now instead: >> >> https://patchwork.kernel.org/patch/11234545/ >> >> ... Stefan, looks like the index.html page is still not there yet, >> although your other patch that includes index.html.in in the sources is >> in the repository now? What's the status here? > > The qemu.git/master docs are built nightly here (index.html is now > visible!): > https://www.qemu.org/docs/master/ > > qemu.org's docs are more useful at the moment since they include the > user documentation in addition to the developer documentation. And I think we want to continue hosting them on qemu.org. Is it possible to theme them under the website branding? Stefan's version includes a banner and background icon so it would be nice to at least fit in with the reset of the site. > It's good that we got ownership of the readthedocs.org account for QEMU. > I don't know if rtd will be capable of building QEMU's hodgepodge of > different documentation systems. It supports Sphinx and Mkdocs but > that's not enough. Yeah at the moment we are just doing the sphinx side of things. AIUI the long term plan is to convert stuff to .rst (at least for the developer docs). > Can we redirect readthedocs to qemu.org? Yeah rtd can label the canonical version of the docs as coming from your own domain and link back (and importantly SEO juice it). > > Stefan -- Alex Bennée
Re: [PATCH 051/104] virtiofsd: Parse flag FUSE_WRITE_KILL_PRIV
> From: Vivek Goyal
>
> Caller can set FUSE_WRITE_KILL_PRIV in write_flags. Parse it and pass it
> to the filesystem.
>
> Signed-off-by: Vivek Goyal
> ---
> tools/virtiofsd/fuse_common.h | 6 +-
> tools/virtiofsd/fuse_lowlevel.c | 4 +++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
> index 147c043bd9..1e8191b7a6 100644
> --- a/tools/virtiofsd/fuse_common.h
> +++ b/tools/virtiofsd/fuse_common.h
> @@ -93,8 +93,12 @@ struct fuse_file_info {
> */
> unsigned int cache_readdir:1;
>
> +/* Indicates that suid/sgid bits should be removed upon write */
> +unsigned int kill_priv:1;
> +
> +
> /** Padding. Reserved for future use*/
> -unsigned int padding:25;
> +unsigned int padding:24;
> unsigned int padding2:32;
>
> /*
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index bd5ca2f157..c8a3b1597a 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -1144,6 +1144,7 @@ static void do_write(fuse_req_t req, fuse_ino_t nodeid,
> memset(&fi, 0, sizeof(fi));
> fi.fh = arg->fh;
> fi.writepage = (arg->write_flags & FUSE_WRITE_CACHE) != 0;
> +fi.kill_priv = !!(arg->write_flags & FUSE_WRITE_KILL_PRIV);
>
> fi.lock_owner = arg->lock_owner;
> fi.flags = arg->flags;
> @@ -1179,7 +1180,8 @@ static void do_write_buf(fuse_req_t req, fuse_ino_t
> nodeid,
> fi.lock_owner = arg->lock_owner;
> fi.flags = arg->flags;
> fi.fh = arg->fh;
> -fi.writepage = arg->write_flags & FUSE_WRITE_CACHE;
> +fi.writepage = !!(arg->write_flags & FUSE_WRITE_CACHE);
> +fi.kill_priv = !!(arg->write_flags & FUSE_WRITE_KILL_PRIV);
>
> if (ibufv->count == 1) {
> assert(!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD));
> --
> 2.23.0
Reviewed-by: Misono Tomohiro
side-note: virtiofs uses write_buf() and therefore do_write() is never called.
How about cleanup the function?
Re: Semihosting, arm, riscv, ppc and common code
Benjamin Herrenschmidt writes: > On Tue, 2020-01-14 at 09:51 +, Alex Bennée wrote: >> > Well, one of the LCA talks wasn't that interesting so I started >> > doing >> > it and am almost done :-) >> > >> > I'll look at doing something for arm, riscv and ppc and send >> > patches >> > once I get it working. >> >> Cool. Are you considering linux-user as well or are these only things >> that make sense for system emulation? > > There seem to be some linux-user stuff in there, I'm mostly considering > whatever ARM does today but we can certainly extend later. Depends on if it is to be used. AFAIK the main users of arm linux user are compiler test cases for M-profile CPUs. > The idea is to make sure ARM, RiscV and POWER use the same protocol and > code base to make picolibc (and others) life easier. Bug compatible > :-) Hmm, I'm not so sure. QEMU tries to emulate real HW although I appreciate that is a somewhat loose definition once you get to things like -M virt and other such SW like abstractions. Is semihosting even going to be a thing on real RiscV/Power silicon? > > Cheers, > Ben. -- Alex Bennée
Re: [qemu-web PATCH] documentation: update links to readthedocs
On Wed, Jan 15, 2020 at 11:56:04AM +, Alex Bennée wrote: > > Stefan Hajnoczi writes: > > > On Tue, Jan 14, 2020 at 12:11:34PM +0100, Thomas Huth wrote: > >> On 13/01/2020 11.35, Alex Bennée wrote: > >> > ..and extemporise a little about their state. > >> > > >> > Signed-off-by: Alex Bennée > >> > --- > >> > documentation.md | 9 ++--- > >> > 1 file changed, 6 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/documentation.md b/documentation.md > >> > index f4ef9f4..55d5db6 100644 > >> > --- a/documentation.md > >> > +++ b/documentation.md > >> > @@ -3,9 +3,12 @@ title: QEMU documentation > >> > permalink: /documentation/ > >> > --- > >> > > >> > -The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be > >> > read online, courtesy of Stefan Weil. > >> > -More documentation is found in the >> > href="https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master";>`docs` > >> > -directory of the QEMU git tree. > >> > +The [QEMU user manual](https://qemu.weilnetz.de/qemu-doc.html) can be > >> > +read online, courtesy of Stefan Weil. There is a partial set of > >> > +[developer documentation](https://qemu.readthedocs.io/en/latest/) > >> > +which is generated from the QEMU git tree. The process of converting > >> > +the rest of the > >> > [`docs`](https://git.qemu.org/?p=qemu.git;a=tree;f=docs;hb=master) > >> > +directory is ongoing. > >> > >> This has a conflict with Stefan's patch to point to our documentation on > >> www.qemu.org now instead: > >> > >> https://patchwork.kernel.org/patch/11234545/ > >> > >> ... Stefan, looks like the index.html page is still not there yet, > >> although your other patch that includes index.html.in in the sources is > >> in the repository now? What's the status here? > > > > The qemu.git/master docs are built nightly here (index.html is now > > visible!): > > https://www.qemu.org/docs/master/ > > > > qemu.org's docs are more useful at the moment since they include the > > user documentation in addition to the developer documentation. > > And I think we want to continue hosting them on qemu.org. Is it possible > to theme them under the website branding? Stefan's version includes a > banner and background icon so it would be nice to at least fit in with > the reset of the site. I did a demo of this a while back: https://www.mail-archive.com/qemu-devel@nongnu.org/msg578110.html Essentially you just need to strip the HTML header down to the content. Then add the jekyll header and run its site generator which will add the new HTML header with theme. In my patches above I committed the ref docs .html files to qemu-web.git but this is not required. They just need to be placed in any directory on the server where jekyll build process runs, so that we can access them when building the static site content. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 2/2] arm/virt/acpi: remove _ADR from devices identified by _HID
On Thu, 19 Dec 2019 14:47:59 +0800
Heyi Guo wrote:
> According to ACPI spec, _ADR should be used for device which is on a
> bus that has a standard enumeration algorithm. It does not make sense
> to have a _ADR object for devices which already have _HID and will be
> enumerated by OSPM.
>
> Signed-off-by: Heyi Guo
Acked-by: Igor Mammedov
>
> ---
> Cc: Shannon Zhao
> Cc: Peter Maydell
> Cc: "Michael S. Tsirkin"
> Cc: Igor Mammedov
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/arm/virt-acpi-build.c | 8
> tests/data/acpi/virt/DSDT | Bin 18449 -> 18426 bytes
> tests/data/acpi/virt/DSDT.memhp | Bin 19786 -> 19763 bytes
> tests/data/acpi/virt/DSDT.numamem | Bin 18449 -> 18426 bytes
> 4 files changed, 8 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9f4c7d1889..be752c0ad8 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -78,11 +78,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const
> MemMapEntry *uart_memmap,
> AML_EXCLUSIVE, &uart_irq, 1));
> aml_append(dev, aml_name_decl("_CRS", crs));
>
> -/* The _ADR entry is used to link this device to the UART described
> - * in the SPCR table, i.e. SPCR.base_address.address == _ADR.
> - */
> -aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base)));
> -
> aml_append(scope, dev);
> }
>
> @@ -170,7 +165,6 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> -aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
> aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> @@ -334,7 +328,6 @@ static void acpi_dsdt_add_gpio(Aml *scope, const
> MemMapEntry *gpio_memmap,
> {
> Aml *dev = aml_device("GPO0");
> aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061")));
> -aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>
> Aml *crs = aml_resource_template();
> @@ -364,7 +357,6 @@ static void acpi_dsdt_add_power_button(Aml *scope)
> {
> Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
> aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> -aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> aml_append(scope, dev);
> }
> diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT
> index
> b5895cb22446860a0b9be3d32ec856feb388be4c..a759ff739a071d5fbf50519a6aea296e5e0f1e0c
> 100644
> GIT binary patch
> delta 72
> zcmbO@f$>*ABbQ6COUN&G1_q{66S<_BT5Bh&t1wzk^tIeLL4lL8ZSqD=gU!!5x$Pt+
> c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8
>
> delta 94
> zcmey>&p2@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao
> tnY@hCfEg&X`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_
>
> diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp
> index
> 69ad844f65d047973a3e55198beecd45a35b8fce..6e5cc61977e4cd24f765fec0693f75a528c144c1
> 100644
> GIT binary patch
> delta 72
> zcmX>#i*fTTMlP3Nmk?uL1_q|eiCof5eHSLGt1wzk^tIeLL4lL8ZSqD=gU!!5U7RH)
> c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>03)CjmjD0&
>
> delta 94
> zcmdlyi}BPfMlP3Nmk=*s1_q}3iCof5t(PXMt1!8;O!Tqj;)r*23F2X3VBp-?s=&$E
> tGkF=O0W(l&^JPwVXL
> diff --git a/tests/data/acpi/virt/DSDT.numamem
> b/tests/data/acpi/virt/DSDT.numamem
> index
> b5895cb22446860a0b9be3d32ec856feb388be4c..a759ff739a071d5fbf50519a6aea296e5e0f1e0c
> 100644
> GIT binary patch
> delta 72
> zcmbO@f$>*ABbQ6COUN&G1_q{66S<_BT5Bh&t1wzk^tIeLL4lL8ZSqD=gU!!5x$Pt+
> c1HyxxIO07#U3dfh0t}oDoEbRcLp@y>07w882mk;8
>
> delta 94
> zcmey>&p2@cBbQ6CONgKc0|V26iCof5J#`b+RhV2^Ci+-%al|{i1o1F1FmP^cRp4ao
> tnY@hCfEg&X`7$S;oxFTNc#soEyoaX?Z-8HbfwO@#16Tu)4E1zj005fm7mWY_
>
Re: [PATCH v2 0/3] exclude hyperv synic sections from vhost
On Mon, Jan 13, 2020 at 05:36:44PM +, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > Hyperv's synic (that we emulate) is a feature that allows the guest > to place some magic (4k) pages of RAM anywhere it likes in GPA. > This confuses vhost's RAM section merging when these pages > land over the top of hugepages. > > Since they're not normal RAM, and they shouldn't have vhost DMAing > into them, exclude them from the vhost set. I still don't think this is correct assessment. These pages are normal RAM, perfectly eligible for DMA and what not. It was a thinko to implement them this way, taking the Hyper-V spec too literally. Among the downsides is the excessive consumption of KVM memslots, and unnecessary large page splits or conflicts with unsplittable ones. I'm working on an alternative approach that doesn't suffer from these issues; struggling to preserve compatibility ATM. Thanks, Roman.
Re: Making QEMU easier for management tools and applications
Christophe de Dinechin writes: > Thanks a bunch. This clarifies a number of my misconceptions about > how this is currently used. Most notably this one: > >> On 15 Jan 2020, at 10:20, Markus Armbruster wrote: >> >>> We don’t want the QAPI to let arbitrary fields of a QOM object >>> be modified, do we? >> >> We already do: QMP command qom-set. If it breaks your guest, you get to >> keep the pieces. > > Ouch. I certainly did not expect that. > > "It is not what you don’t know that kills you, it is what you know that ain’t > so". Do we have a legitimate use for qom-set right now? Hmm, let's check libvirt... aha: * qemuMonitorJSONSetMemoryStatsPeriod() uses it to control virtio-balloon's guest-stats-polling-interval property, in accordance with docs/virtio-balloon-stats.txt. * qemuMonitorJSONSetIOThread() uses it to control iothread's properties poll-max-ns, poll-grow, poll-shrink. Their use with -object is documented (in qemu-options.hx), their use with qom-set is not. Oh well. http://dreamsongs.com/RiseOfWorseIsBetter.html >>> >>> You know that I positively hate this ;-) >> >> It's been a tough lesson for me, too. > > Not sure I can call it a “lesson”. Much like a philosophy to fight against, > IMO. The tough lesson isn't philosophy, it's an observation: "I believe that worse-is-better [...] has better survival characteristics than the-right-thing". >>> Well, I guess we can expand the schema. #ILoveJSON. >> >> Basing the QAPI language on JSON was a poor choice. Not sure that's >> fixable at a reasonable cost. > > Well, at least now I have a slightly better understanding of the related costs > and trade-offs. Thanks a lot for explaining. You're welcome!
Re: Making QEMU easier for management tools and applications
On Wed, Jan 15, 2020 at 01:15:17PM +0100, Markus Armbruster wrote: > Christophe de Dinechin writes: > > > Thanks a bunch. This clarifies a number of my misconceptions about > > how this is currently used. Most notably this one: > > > >> On 15 Jan 2020, at 10:20, Markus Armbruster wrote: > >> > >>> We don’t want the QAPI to let arbitrary fields of a QOM object > >>> be modified, do we? > >> > >> We already do: QMP command qom-set. If it breaks your guest, you get to > >> keep the pieces. > > > > Ouch. I certainly did not expect that. > > > > "It is not what you don’t know that kills you, it is what you know that > > ain’t so". > > Do we have a legitimate use for qom-set right now? Hmm, let's check > libvirt... aha: > > * qemuMonitorJSONSetMemoryStatsPeriod() uses it to control > virtio-balloon's guest-stats-polling-interval property, in accordance > with docs/virtio-balloon-stats.txt. > > * qemuMonitorJSONSetIOThread() uses it to control iothread's properties > poll-max-ns, poll-grow, poll-shrink. Their use with -object is > documented (in qemu-options.hx), their use with qom-set is not. > > Oh well. Libvirt is of course happy to switch to something else instead of qom-set for these features if QEMU wants to provide a safer alternative. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v3 0/4] qmp: Optionally run handlers in coroutines
Some QMP command handlers can block the main loop for a relatively long time, for example because they perform some I/O. This is quite nasty. Allowing such handlers to run in a coroutine where they can yield (and therefore release the BQL) while waiting for an event such as I/O completion solves the problem. This series adds the infrastructure to allow this and switches block_resize to run in a coroutine as a first example. This is an alternative solution to Marc-André's "monitor: add asynchronous command type" series. v3: - Fix race between monitor thread and dispatcher that could schedule the dispatcher coroutine twice if a second requests comes in before the dispatcher can wake up [Patchew] v2: - Fix typo in a commit message [Eric] - Use hyphen instead of underscore for the test command [Eric] - Mark qmp_block_resize() as coroutine_fn [Stefan] Kevin Wolf (4): qapi: Add a 'coroutine' flag for commands vl: Initialise main loop earlier qmp: Move dispatcher to a coroutine block: Mark 'block_resize' as coroutine qapi/block-core.json| 3 +- tests/qapi-schema/qapi-schema-test.json | 1 + docs/devel/qapi-code-gen.txt| 4 ++ include/qapi/qmp/dispatch.h | 3 + monitor/monitor-internal.h | 5 +- blockdev.c | 6 +- monitor/monitor.c | 24 --- monitor/qmp.c | 83 - qapi/qmp-dispatch.c | 38 ++- tests/test-qmp-cmds.c | 4 ++ vl.c| 10 +-- scripts/qapi/commands.py| 17 +++-- scripts/qapi/doc.py | 2 +- scripts/qapi/expr.py| 4 +- scripts/qapi/introspect.py | 2 +- scripts/qapi/schema.py | 9 ++- tests/qapi-schema/qapi-schema-test.out | 2 + tests/qapi-schema/test-qapi.py | 7 ++- 18 files changed, 158 insertions(+), 66 deletions(-) -- 2.20.1
[PATCH v3 3/4] qmp: Move dispatcher to a coroutine
This moves the QMP dispatcher to a coroutine and runs all QMP command
handlers that declare 'coroutine': true in coroutine context so they
can avoid blocking the main loop while doing I/O or waiting for other
events.
For commands that are not declared safe to run in a coroutine, the
dispatcher drops out of coroutine context by calling the QMP command
handler from a bottom half.
Signed-off-by: Kevin Wolf
Reviewed-by: Marc-André Lureau
---
include/qapi/qmp/dispatch.h | 2 +
monitor/monitor-internal.h | 5 ++-
monitor/monitor.c | 24 +++
monitor/qmp.c | 83 +++--
qapi/qmp-dispatch.c | 38 -
5 files changed, 111 insertions(+), 41 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index d6ce9efc8e..d6d5443391 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -30,6 +30,8 @@ typedef enum QmpCommandOptions
typedef struct QmpCommand
{
const char *name;
+/* Runs in coroutine context if QCO_COROUTINE is set, except for OOB
+ * commands */
QmpCommandFunc *fn;
QmpCommandOptions options;
QTAILQ_ENTRY(QmpCommand) node;
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index d78f5ca190..7389b6a56c 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -154,7 +154,8 @@ static inline bool monitor_is_qmp(const Monitor *mon)
typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
extern IOThread *mon_iothread;
-extern QEMUBH *qmp_dispatcher_bh;
+extern Coroutine *qmp_dispatcher_co;
+extern bool qmp_dispatcher_co_busy;
extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
extern QemuMutex monitor_lock;
extern MonitorList mon_list;
@@ -172,7 +173,7 @@ void monitor_fdsets_cleanup(void);
void qmp_send_response(MonitorQMP *mon, const QDict *rsp);
void monitor_data_destroy_qmp(MonitorQMP *mon);
-void monitor_qmp_bh_dispatcher(void *data);
+void coroutine_fn monitor_qmp_dispatcher_co(void *data);
int get_monitor_def(int64_t *pval, const char *name);
void help_cmd(Monitor *mon, const char *name);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 12898b6448..c72763fa4e 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -53,8 +53,9 @@ typedef struct {
/* Shared monitor I/O thread */
IOThread *mon_iothread;
-/* Bottom half to dispatch the requests received from I/O thread */
-QEMUBH *qmp_dispatcher_bh;
+/* Coroutine to dispatch the requests received from I/O thread */
+Coroutine *qmp_dispatcher_co;
+bool qmp_dispatcher_co_busy;
/* Protects mon_list, monitor_qapi_event_state, monitor_destroyed. */
QemuMutex monitor_lock;
@@ -579,9 +580,16 @@ void monitor_cleanup(void)
}
qemu_mutex_unlock(&monitor_lock);
-/* QEMUBHs needs to be deleted before destroying the I/O thread */
-qemu_bh_delete(qmp_dispatcher_bh);
-qmp_dispatcher_bh = NULL;
+/* The dispatcher needs to stop before destroying the I/O thread */
+if (!atomic_mb_read(&qmp_dispatcher_co_busy)) {
+aio_co_schedule(iohandler_get_aio_context(), qmp_dispatcher_co);
+qmp_dispatcher_co = NULL;
+}
+
+AIO_WAIT_WHILE(qemu_get_aio_context(),
+ (aio_bh_poll(iohandler_get_aio_context()),
+atomic_mb_read(&qmp_dispatcher_co_busy)));
+
if (mon_iothread) {
iothread_destroy(mon_iothread);
mon_iothread = NULL;
@@ -604,9 +612,9 @@ void monitor_init_globals_core(void)
* have commands assuming that context. It would be nice to get
* rid of those assumptions.
*/
-qmp_dispatcher_bh = aio_bh_new(iohandler_get_aio_context(),
- monitor_qmp_bh_dispatcher,
- NULL);
+qmp_dispatcher_co = qemu_coroutine_create(monitor_qmp_dispatcher_co, NULL);
+atomic_mb_set(&qmp_dispatcher_co_busy, true);
+aio_co_schedule(iohandler_get_aio_context(), qmp_dispatcher_co);
}
QemuOptsList qemu_mon_opts = {
diff --git a/monitor/qmp.c b/monitor/qmp.c
index b67a8e7d1f..f29a8fe497 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -133,6 +133,8 @@ static void monitor_qmp_respond(MonitorQMP *mon, QDict *rsp)
}
}
+/* Runs outside of coroutine context for OOB commands, but in coroutine context
+ * for everything else. */
static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
{
Monitor *old_mon;
@@ -211,43 +213,62 @@ static QMPRequest
*monitor_qmp_requests_pop_any_with_lock(void)
return req_obj;
}
-void monitor_qmp_bh_dispatcher(void *data)
+void coroutine_fn monitor_qmp_dispatcher_co(void *data)
{
-QMPRequest *req_obj = monitor_qmp_requests_pop_any_with_lock();
+QMPRequest *req_obj = NULL;
QDict *rsp;
bool need_resume;
MonitorQMP *mon;
-if (!req_obj) {
-return;
-}
+while (true) {
+assert(atomic_mb_read(&qmp_dispatcher_co_busy) == true);
+
+while (!(req_obj = m
[PATCH v3 2/4] vl: Initialise main loop earlier
We want to be able to use qemu_aio_context in the monitor
initialisation.
Signed-off-by: Kevin Wolf
Reviewed-by: Marc-André Lureau
---
vl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/vl.c b/vl.c
index 86474a55c9..4c79a00857 100644
--- a/vl.c
+++ b/vl.c
@@ -2903,6 +2903,11 @@ int main(int argc, char **argv, char **envp)
runstate_init();
precopy_infrastructure_init();
postcopy_infrastructure_init();
+
+if (qemu_init_main_loop(&main_loop_err)) {
+error_report_err(main_loop_err);
+exit(1);
+}
monitor_init_globals();
if (qcrypto_init(&err) < 0) {
@@ -3817,11 +3822,6 @@ int main(int argc, char **argv, char **envp)
qemu_unlink_pidfile_notifier.notify = qemu_unlink_pidfile;
qemu_add_exit_notifier(&qemu_unlink_pidfile_notifier);
-if (qemu_init_main_loop(&main_loop_err)) {
-error_report_err(main_loop_err);
-exit(1);
-}
-
#ifdef CONFIG_SECCOMP
olist = qemu_find_opts_err("sandbox", NULL);
if (olist) {
--
2.20.1
Re: Making QEMU easier for management tools and applications
Marc-André Lureau writes: > Hi > > On Wed, Jan 15, 2020 at 1:21 PM Markus Armbruster wrote: >> >> Christophe de Dinechin writes: >> >> >> To make this worthwhile, we'd have to replace dynamic QOM properties by >> >> static ones when possible. Monumental task. >> > >> > I’m sure you are right, but it’s hard for me to evaluate, given how >> > many ways there are to access an object. Naively, grepping for >> > set_prop and for new_with_prop does not give me that many hits. >> >> Look for object_property_add*(). Some 450 hits. > > fwiw, I have started tackling that. Laudable. > Easy first step is to move all QDev properties to class properties, > and this is done in : > https://patchew.org/QEMU/20200110153039.1379601-1-marcandre.lur...@redhat.com/ Easy because qdev properties are declarative by design. Imperative came in when they got rebased onto QOM. > Moving from instance to class properties is straightforward many times > (when the property is unconditonally added in instance init for > example). There are a few complicated cases though. > > To me, the most problematic is reviewer-time and willingness to do > such low-benefits changes. Understand. >> Basing the QAPI language on JSON was a poor choice. Aggravated by immediately extending it in ways that defeat all common JSON tools. >> Not sure that's >> fixable at a reasonable cost. > > Translating it to another language should be relatively easy, but to what? The QAPI language is layered on top of JSON (see docs/devel/qapi-code-gen.txt section "Schema syntax"). Swapping out JSON for another language that is at least as expressive is an relatively simple change. Obvious candidates: s-expressions, TOML, YAML. The latter is far too complex for my taste. The expensive part is dealing with the conflicts during the transition, and having everybody relearn QAPI schema syntax.
[PATCH v3 1/4] qapi: Add a 'coroutine' flag for commands
This patch adds a new 'coroutine' flag to QMP command definitions that
tells the QMP dispatcher that the command handler is safe to be run in a
coroutine.
Signed-off-by: Kevin Wolf
Reviewed-by: Marc-André Lureau
---
tests/qapi-schema/qapi-schema-test.json | 1 +
docs/devel/qapi-code-gen.txt| 4
include/qapi/qmp/dispatch.h | 1 +
tests/test-qmp-cmds.c | 4
scripts/qapi/commands.py| 17 +++--
scripts/qapi/doc.py | 2 +-
scripts/qapi/expr.py| 4 ++--
scripts/qapi/introspect.py | 2 +-
scripts/qapi/schema.py | 9 ++---
tests/qapi-schema/qapi-schema-test.out | 2 ++
tests/qapi-schema/test-qapi.py | 7 ---
11 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 9abf175fe0..1a850fe171 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -147,6 +147,7 @@
'returns': 'UserDefTwo' }
{ 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
+{ 'command': 'coroutine-cmd', 'data': {}, 'coroutine': true }
# Returning a non-dictionary requires a name from the whitelist
{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 45c93a43cc..753f6711d3 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -457,6 +457,7 @@ Syntax:
'*gen': false,
'*allow-oob': true,
'*allow-preconfig': true,
+'*coroutine': true,
'*if': COND,
'*features': FEATURES }
@@ -581,6 +582,9 @@ before the machine is built. It defaults to false. For
example:
QMP is available before the machine is built only when QEMU was
started with --preconfig.
+Member 'coroutine' tells the QMP dispatcher whether the command handler
+is safe to be run in a coroutine. It defaults to false.
+
The optional 'if' member specifies a conditional. See "Configuring
the schema" below for more on this.
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 9aa426a398..d6ce9efc8e 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -24,6 +24,7 @@ typedef enum QmpCommandOptions
QCO_NO_SUCCESS_RESP = (1U << 0),
QCO_ALLOW_OOB = (1U << 1),
QCO_ALLOW_PRECONFIG = (1U << 2),
+QCO_COROUTINE = (1U << 3),
} QmpCommandOptions;
typedef struct QmpCommand
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 27b0afe55a..e2f71e42a1 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -34,6 +34,10 @@ void qmp_cmd_success_response(Error **errp)
{
}
+void qmp_coroutine_cmd(Error **errp)
+{
+}
+
Empty2 *qmp_user_def_cmd0(Error **errp)
{
return g_new0(Empty2, 1);
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index ab98e504f3..97e51401f1 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -15,6 +15,7 @@ See the COPYING file in the top-level directory.
from qapi.common import *
from qapi.gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext
+from typing import List
def gen_command_decl(name, arg_type, boxed, ret_type):
@@ -194,8 +195,9 @@ out:
return ret
-def gen_register_command(name, success_response, allow_oob, allow_preconfig):
-options = []
+def gen_register_command(name: str, success_response: bool, allow_oob: bool,
+ allow_preconfig: bool, coroutine: bool) -> str:
+options = [] # type: List[str]
if not success_response:
options += ['QCO_NO_SUCCESS_RESP']
@@ -203,18 +205,20 @@ def gen_register_command(name, success_response,
allow_oob, allow_preconfig):
options += ['QCO_ALLOW_OOB']
if allow_preconfig:
options += ['QCO_ALLOW_PRECONFIG']
+if coroutine:
+options += ['QCO_COROUTINE']
if not options:
options = ['QCO_NO_OPTIONS']
-options = " | ".join(options)
+options_str = " | ".join(options)
ret = mcgen('''
qmp_register_command(cmds, "%(name)s",
qmp_marshal_%(c_name)s, %(opts)s);
''',
name=name, c_name=c_name(name),
-opts=options)
+opts=options_str)
return ret
@@ -278,7 +282,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
success_response, boxed, allow_oob, allow_preconfig,
- features):
+ coroutine, features):
if not gen:
return
# FIXME: If T is a user-defined type, the user is responsible
@@ -296,7 +300,8 @@ void %(c_prefix)sqmp_in
[PATCH v3 4/4] block: Mark 'block_resize' as coroutine
block_resize is safe to run in a coroutine, so use it as an example for
the new 'coroutine': true annotation in the QAPI schema.
Signed-off-by: Kevin Wolf
Reviewed-by: Marc-André Lureau
---
qapi/block-core.json | 3 ++-
blockdev.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ff5e5edaf..1dbb2a9901 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1341,7 +1341,8 @@
{ 'command': 'block_resize',
'data': { '*device': 'str',
'*node-name': 'str',
-'size': 'int' } }
+'size': 'int' },
+ 'coroutine': true }
##
# @NewImageMode:
diff --git a/blockdev.c b/blockdev.c
index 8e029e9c01..b5e5d1e072 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3161,9 +3161,9 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
aio_context_release(aio_context);
}
-void qmp_block_resize(bool has_device, const char *device,
- bool has_node_name, const char *node_name,
- int64_t size, Error **errp)
+void coroutine_fn qmp_block_resize(bool has_device, const char *device,
+ bool has_node_name, const char *node_name,
+ int64_t size, Error **errp)
{
Error *local_err = NULL;
BlockBackend *blk = NULL;
--
2.20.1
Re: [PATCH v2 0/4] qmp: Optionally run handlers in coroutines
Am 14.01.2020 um 21:41 hat no-re...@patchew.org geschrieben: > Patchew URL: https://patchew.org/QEMU/20200114182735.5553-1-kw...@redhat.com/ > > Hi, > > This series failed the docker-quick@centos7 build test. Please find the > testing commands and > their output below. If you have Docker installed, you can probably reproduce > it > locally. > > === TEST SCRIPT BEGIN === > #!/bin/bash > make docker-image-centos7 V=1 NETWORK=1 > time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 > === TEST SCRIPT END === > > aio_co_schedule: Co-routine was already scheduled in 'aio_co_schedule' > Broken pipe > /tmp/qemu-test/src/tests/qtest/libqtest.c:149: kill_qemu() detected QEMU > death from signal 6 (Aborted) (core dumped) > ERROR - too few tests run (expected 4, got 1) > make: *** [check-qtest-aarch64] Error 1 > make: *** Waiting for unfinished jobs This was a real bug in the series, v3 incoming. Kevin
Re: Semihosting, arm, riscv, ppc and common code
> On 15 Jan 2020, at 14:01, Alex Bennée wrote: > > ... AFAIK the main users of arm linux user > are compiler test cases for M-profile CPUs. I confirm, generally unit tests. But not necessarily, I used QEMU as the main development platform for the Cortex-M port of µOS++, a C/C++ framework/RTOS, and it was very convenient. > ... Is semihosting even > going to be a thing on real RiscV/Power silicon? Even for Arm it is a thing of debuggers and development environments, not of silicon itself, since it requires no special hardware support, a BRK instruction is enough. From a software point of view it would be beneficial to use the same semihosting protocol on other non-Arm architectures too, even if it is far from perfect. I pushed hard for adopting the Arm semihosting for RISC-V, and even contributed the semihosting code for OpenOCD RISC-V, but officially the only mention of semihosting in the RISC-V specs is in the notes sections, where it is mentioned how to use the BRK instruction to invoke semihosting calls (RISC-V has a single BREAK instructions, and to make it unique it must be surrounded by some hint NOPs). Regards, Liviu
[PATCH v7 04/11] hw/core: add Resettable support to BusClass and DeviceClass
This commit adds support of Resettable interface to buses and devices:
+ ResettableState structure is added in the Bus/Device state
+ Resettable methods are implemented.
+ device/bus_is_in_reset function defined
This commit allows to transition the objects to the new
multi-phase interface without changing the reset behavior at all.
Object single reset method can be split into the 3 different phases
but the 3 phases are still executed in a row for a given object.
>From the qdev/qbus reset api point of view, nothing is changed.
qdev_reset_all() and qbus_reset_all() are not modified as well as
device_legacy_reset().
Transition of an object must be done from parent class to child class.
Care has been taken to allow the transition of a parent class
without requiring the child classes to be transitioned at the same
time. Note that SysBus and SysBusDevice class do not need any transition
because they do not override the legacy reset method.
Signed-off-by: Damien Hedde
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
---
include/hw/qdev-core.h | 27
hw/core/bus.c | 97 ++
hw/core/qdev.c | 93
tests/Makefile.include | 1 +
4 files changed, 218 insertions(+)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b8341b0fb0..1b4b420617 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -5,6 +5,7 @@
#include "qemu/bitmap.h"
#include "qom/object.h"
#include "hw/hotplug.h"
+#include "hw/resettable.h"
enum {
DEV_NVECTORS_UNSPECIFIED = -1,
@@ -117,6 +118,11 @@ typedef struct DeviceClass {
bool hotpluggable;
/* callbacks */
+/*
+ * Reset method here is deprecated and replaced by methods in the
+ * resettable class interface to implement a multi-phase reset.
+ * TODO: remove once every reset callback is unused
+ */
DeviceReset reset;
DeviceRealize realize;
DeviceUnrealize unrealize;
@@ -141,6 +147,7 @@ struct NamedGPIOList {
/**
* DeviceState:
* @realized: Indicates whether the device has been fully constructed.
+ * @reset: ResettableState for the device; handled by Resettable interface.
*
* This structure should not be accessed directly. We declare it here
* so that it can be embedded in individual device state structures.
@@ -163,6 +170,7 @@ struct DeviceState {
int num_child_bus;
int instance_id_alias;
int alias_required_for_version;
+ResettableState reset;
};
struct DeviceListener {
@@ -215,6 +223,7 @@ typedef struct BusChild {
/**
* BusState:
* @hotplug_handler: link to a hotplug handler associated with bus.
+ * @reset: ResettableState for the bus; handled by Resettable interface.
*/
struct BusState {
Object obj;
@@ -226,6 +235,7 @@ struct BusState {
int num_children;
QTAILQ_HEAD(, BusChild) children;
QLIST_ENTRY(BusState) sibling;
+ResettableState reset;
};
/**
@@ -412,6 +422,18 @@ void qdev_reset_all_fn(void *opaque);
void qbus_reset_all(BusState *bus);
void qbus_reset_all_fn(void *opaque);
+/**
+ * device_is_in_reset:
+ * Return true if the device @dev is currently being reset.
+ */
+bool device_is_in_reset(DeviceState *dev);
+
+/**
+ * bus_is_in_reset:
+ * Return true if the bus @bus is currently being reset.
+ */
+bool bus_is_in_reset(BusState *bus);
+
/* This should go away once we get rid of the NULL bus hack */
BusState *sysbus_get_default(void);
@@ -433,6 +455,11 @@ void qdev_machine_init(void);
*/
void device_legacy_reset(DeviceState *dev);
+/**
+ * device_class_set_parent_reset:
+ * TODO: remove the function when DeviceClass's reset method
+ * is not used anymore.
+ */
void device_class_set_parent_reset(DeviceClass *dc,
DeviceReset dev_reset,
DeviceReset *parent_reset);
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 7f3d2a3dbd..2698f715bd 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -68,6 +68,28 @@ int qbus_walk_children(BusState *bus,
return 0;
}
+bool bus_is_in_reset(BusState *bus)
+{
+return resettable_is_in_reset(OBJECT(bus));
+}
+
+static ResettableState *bus_get_reset_state(Object *obj)
+{
+BusState *bus = BUS(obj);
+return &bus->reset;
+}
+
+static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
+void *opaque, ResetType type)
+{
+BusState *bus = BUS(obj);
+BusChild *kid;
+
+QTAILQ_FOREACH(kid, &bus->children, sibling) {
+cb(OBJECT(kid->child), opaque, type);
+}
+}
+
static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
{
const char *typename = object_get_typename(OBJECT(bus));
@@ -199,12 +221,83 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev)
return g_strdup(object_get_typename(OBJECT(dev)));
}
+/**
+ * bus_phases_reset:
+ * Transition reset method for buses to allow moving
+
[PATCH v7 00/11] Multi-phase reset mechanism
Hi all, The purpose of this series is to split the current reset procedure into multiple phases. This will help to solve some ordering difficulties we have during reset. This is a ready to merge version. I've rebased on master and followed Richard's remarks on v6. All patches have been reviewed by. This series adds resettable interface and transitions base Device and Bus classes (sysbus subclasses are ok too). It provides new reset functions but does not switch anymore the old functions (device_reset() and qdev/qbus_reset_all()) to resettable interface. These functions keep the exact same behavior as before. The series also transition the main reset handlers registration which has no impact until devices and buses are transitioned. The series is organized as follows: Patch 1 is unmodified. Patch 2 adds some utility trace events. Patches 3 to 8 adds resettable api in devices and buses. Patch 9 adds some documentation. Patches 10 and 11 transition the call sites of qemu_register_reset(qdev/qbus_reset_all_fn, ...). After this series, the plan is then to transition devices, buses and legacy reset call sites. Devices and buses have to be transitioned from mother class to daughter classes order but until the final (daughter) class is transitioned, old monolitic reset behavior will be kept for this class. Thanks, Damien v7: + patch 1 - new device_reset occurrence (rebase) + patch 3 - ResettablePhases un-nest (Richard) - warnings (Richard) + patch 7 - inline resettable_state_clear (Richard) v6: https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04664.html + patch 1 - update (microvm machine) (rebase) + patch 2 - added more info in trace_qdev_update_parent_bus (Philippe) + patch 3 - typos (Peter) - missing gpl header in header (Peter) - clarify global entry_in_progress flag (Peter) + patch 4 - parent/child terminology (Peter) + patch 5 - typos (Peter) - comment about global exit_in_progress flag (Peter) + patch 6 - update list of qdev_set_parent_bus() call sites (rebase) + patch 7 - clear reset state in realize before doing the hotplug reset + patch 9 - added entry in index.rst - fix commit message and various improvements (Peter) - do parent phase method before child actions (Peter) - clarify "Polling the reset state" paragraph + patch 12&13 - removed (follow-up series to cleanup the raspi sd) v5: https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04730.html + various improvement in the resettable interface (regarding transition, robustness and several reset types) + better handling of transition from legacy reset to resettable + device hotplug and parent bus 'hot' change support + improved doc with examples and converted to rst format v4: https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg04359.html Damien Hedde (11): add device_legacy_reset function to prepare for reset api change hw/core/qdev: add trace events to help with resettable transition hw/core: create Resettable QOM interface hw/core: add Resettable support to BusClass and DeviceClass hw/core/resettable: add support for changing parent hw/core/qdev: handle parent bus change regarding resettable hw/core/qdev: update hotplug reset regarding resettable hw/core: deprecate old reset functions and introduce new ones docs/devel/reset.rst: add doc about Resettable interface vl: replace deprecated qbus_reset_all registration hw/s390x/ipl: replace deprecated qdev_reset_all registration docs/devel/index.rst | 1 + docs/devel/reset.rst | 289 + Makefile.objs| 1 + include/hw/qdev-core.h | 58 +++- include/hw/resettable.h | 247 hw/audio/intel-hda.c | 2 +- hw/core/bus.c| 102 + hw/core/qdev.c | 160 +++-- hw/core/resettable.c | 301 +++ hw/hyperv/hyperv.c | 2 +- hw/i386/microvm.c| 2 +- hw/i386/pc.c | 2 +- hw/ide/microdrive.c | 8 +- hw/intc/spapr_xive.c | 2 +- hw/ppc/pnv_psi.c | 4 +- hw/ppc/spapr_pci.c | 2 +- hw/ppc/spapr_vio.c | 2 +- hw/s390x/ipl.c | 10 +- hw/s390x/s390-pci-inst.c | 2 +- hw/scsi/vmw_pvscsi.c | 2 +- hw/sd/omap_mmc.c | 2 +- hw/sd/pl181.c| 2 +- vl.c | 10 +- hw/core/Makefile.objs| 1 + hw/core/trace-events | 27 tests/Makefile.include | 1 + 26 files changed, 1211 insertions(+), 31 deletions(-) create mode 100644 docs/devel/reset.rst create mode 100644 include/hw/resettable.h create mode 100644 hw/core/resettable.c -- 2.24.1
[PATCH v7 02/11] hw/core/qdev: add trace events to help with resettable transition
Adds trace events to reset procedure and when updating the parent
bus of a device.
Signed-off-by: Damien Hedde
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
---
hw/core/qdev.c | 29 ++---
hw/core/trace-events | 9 +
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 8c0c8284c8..5cb03136b5 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -38,6 +38,7 @@
#include "hw/boards.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
+#include "trace.h"
bool qdev_hotplug = false;
static bool qdev_hot_added = false;
@@ -98,7 +99,11 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
bool replugging = dev->parent_bus != NULL;
if (replugging) {
-/* Keep a reference to the device while it's not plugged into
+trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)),
+dev->parent_bus, object_get_typename(OBJECT(dev->parent_bus)),
+OBJECT(bus), object_get_typename(OBJECT(bus)));
+/*
+ * Keep a reference to the device while it's not plugged into
* any bus, to avoid it potentially evaporating when it is
* dereffed in bus_remove_child().
*/
@@ -296,6 +301,18 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
return hotplug_ctrl;
}
+static int qdev_prereset(DeviceState *dev, void *opaque)
+{
+trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev)));
+return 0;
+}
+
+static int qbus_prereset(BusState *bus, void *opaque)
+{
+trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus)));
+return 0;
+}
+
static int qdev_reset_one(DeviceState *dev, void *opaque)
{
device_legacy_reset(dev);
@@ -306,6 +323,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque)
static int qbus_reset_one(BusState *bus, void *opaque)
{
BusClass *bc = BUS_GET_CLASS(bus);
+trace_qbus_reset(bus, object_get_typename(OBJECT(bus)));
if (bc->reset) {
bc->reset(bus);
}
@@ -314,7 +332,9 @@ static int qbus_reset_one(BusState *bus, void *opaque)
void qdev_reset_all(DeviceState *dev)
{
-qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
+trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev)));
+qdev_walk_children(dev, qdev_prereset, qbus_prereset,
+ qdev_reset_one, qbus_reset_one, NULL);
}
void qdev_reset_all_fn(void *opaque)
@@ -324,7 +344,9 @@ void qdev_reset_all_fn(void *opaque)
void qbus_reset_all(BusState *bus)
{
-qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
+trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus)));
+qbus_walk_children(bus, qdev_prereset, qbus_prereset,
+ qdev_reset_one, qbus_reset_one, NULL);
}
void qbus_reset_all_fn(void *opaque)
@@ -1131,6 +1153,7 @@ void device_legacy_reset(DeviceState *dev)
{
DeviceClass *klass = DEVICE_GET_CLASS(dev);
+trace_qdev_reset(dev, object_get_typename(OBJECT(dev)));
if (klass->reset) {
klass->reset(dev);
}
diff --git a/hw/core/trace-events b/hw/core/trace-events
index fe47a9c8cb..a375aa88a4 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -1,2 +1,11 @@
# loader.c
loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool isrom)
"%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d"
+
+# qdev.c
+qdev_reset(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_update_parent_bus(void *obj, const char *objtype, void *oldp, const char
*oldptype, void *newp, const char *newptype) "obj=%p(%s) old_parent=%p(%s)
new_parent=%p(%s)"
--
2.24.1
[PATCH v7 01/11] add device_legacy_reset function to prepare for reset api change
Provide a temporary device_legacy_reset function doing what
device_reset does to prepare for the transition with Resettable
API.
All occurrence of device_reset in the code tree are also replaced
by device_legacy_reset.
The new resettable API has different prototype and semantics
(resetting child buses as well as the specified device). Subsequent
commits will make the changeover for each call site individually; once
that is complete device_legacy_reset() will be removed.
Signed-off-by: Damien Hedde
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
Acked-by: David Gibson
Acked-by: Cornelia Huck
---
v7 update: added new occurence due to rebase
Cc: Gerd Hoffmann
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Richard Henderson
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: John Snow
Cc: "Cédric Le Goater"
Cc: Collin Walling
Cc: Cornelia Huck
Cc: David Hildenbrand
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Dmitry Fleytman
Cc: Fam Zheng
---
include/hw/qdev-core.h | 4 ++--
hw/audio/intel-hda.c | 2 +-
hw/core/qdev.c | 6 +++---
hw/hyperv/hyperv.c | 2 +-
hw/i386/microvm.c| 2 +-
hw/i386/pc.c | 2 +-
hw/ide/microdrive.c | 8
hw/intc/spapr_xive.c | 2 +-
hw/ppc/pnv_psi.c | 4 ++--
hw/ppc/spapr_pci.c | 2 +-
hw/ppc/spapr_vio.c | 2 +-
hw/s390x/s390-pci-inst.c | 2 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/sd/omap_mmc.c | 2 +-
hw/sd/pl181.c| 2 +-
15 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1518495b1e..b8341b0fb0 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -427,11 +427,11 @@ char *qdev_get_own_fw_dev_path_from_handler(BusState
*bus, DeviceState *dev);
void qdev_machine_init(void);
/**
- * @device_reset
+ * device_legacy_reset:
*
* Reset a single device (by calling the reset method).
*/
-void device_reset(DeviceState *dev);
+void device_legacy_reset(DeviceState *dev);
void device_class_set_parent_reset(DeviceClass *dc,
DeviceReset dev_reset,
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 6ecd383540..27b71c57cf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1087,7 +1087,7 @@ static void intel_hda_reset(DeviceState *dev)
QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
DeviceState *qdev = kid->child;
cdev = HDA_CODEC_DEVICE(qdev);
-device_reset(DEVICE(cdev));
+device_legacy_reset(DEVICE(cdev));
d->state_sts |= (1 << cdev->cad);
}
intel_hda_update_irq(d);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9f1753f5cf..8c0c8284c8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -298,7 +298,7 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
static int qdev_reset_one(DeviceState *dev, void *opaque)
{
-device_reset(dev);
+device_legacy_reset(dev);
return 0;
}
@@ -895,7 +895,7 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
}
}
if (dev->hotplugged) {
-device_reset(dev);
+device_legacy_reset(dev);
}
dev->pending_deleted_event = false;
@@ -1127,7 +1127,7 @@ void device_class_set_parent_unrealize(DeviceClass *dc,
dc->unrealize = dev_unrealize;
}
-void device_reset(DeviceState *dev)
+void device_legacy_reset(DeviceState *dev)
{
DeviceClass *klass = DEVICE_GET_CLASS(dev);
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index da8ce82725..8ca3706f5b 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -140,7 +140,7 @@ void hyperv_synic_reset(CPUState *cs)
SynICState *synic = get_synic(cs);
if (synic) {
-device_reset(DEVICE(synic));
+device_legacy_reset(DEVICE(synic));
}
}
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 827ce29e58..d23485108d 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -370,7 +370,7 @@ static void microvm_machine_reset(MachineState *machine)
cpu = X86_CPU(cs);
if (cpu->apic_state) {
-device_reset(cpu->apic_state);
+device_legacy_reset(cpu->apic_state);
}
}
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8054bc4147..91c7fdaab1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1877,7 +1877,7 @@ static void pc_machine_reset(MachineState *machine)
cpu = X86_CPU(cs);
if (cpu->apic_state) {
-device_reset(cpu->apic_state);
+device_legacy_reset(cpu->apic_state);
}
}
}
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index b0272ea14b..6b30e36ed8 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -173,7 +173,7 @@ static void md_attr_write(PCMCIACardState *card, uint32_t
at, uint8_t value)
case 0x00: /* Configuration Option Register */
s->opt = value & 0xcf;
[PATCH v7 07/11] hw/core/qdev: update hotplug reset regarding resettable
This commit make use of the resettable API to reset the device being
hotplugged when it is realized. Also it ensures it is put in a reset
state coherent with the parent it is plugged into.
Note that there is a difference in the reset. Instead of resetting
only the hotplugged device, we reset also its subtree (switch to
resettable API). This is not expected to be a problem because
sub-buses are just realized too. If a hotplugged device has any
sub-buses it is logical to reset them too at this point.
The recently added should_be_hidden and PCI's partially_hotplugged
mechanisms do not interfere with realize operation:
+ In the should_be_hidden use case, device creation is
delayed.
+ The partially_hotplugged mechanism prevents a device to be
unplugged and unrealized from qdev POV and unrealized.
Signed-off-by: Damien Hedde
Reviewed-by: Richard Henderson
---
v7 update: inline resettable_state_clear()
v6 update: clear the reset state before doing any reset. Although it
is apparently highly improbable that a device be realized again after
being unrealized. See here for a discussion about this point and the
partially_hotplugged/shoud_be_hidden cases.
https://lists.nongnu.org/archive/html/qemu-devel/2019-11/msg04897.html
---
include/hw/resettable.h | 11 +++
hw/core/qdev.c | 15 ++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
index 3c87ab86c7..3f90da5b5b 100644
--- a/include/hw/resettable.h
+++ b/include/hw/resettable.h
@@ -153,6 +153,17 @@ struct ResettableState {
bool exit_phase_in_progress;
};
+/**
+ * resettable_state_clear:
+ * Clear the state. It puts the state to the initial (zeroed) state required
+ * to reuse an object. Typically used in realize step of base classes
+ * implementing the interface.
+ */
+static inline void resettable_state_clear(ResettableState *state)
+{
+memset(state, 0, sizeof(ResettableState));
+}
+
/**
* resettable_reset:
* Trigger a reset on an object @obj of type @type. @obj must implement
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 310b87e25a..5fac4cd8fc 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -937,6 +937,12 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
}
}
+/*
+ * Clear the reset state, in case the object was previously unrealized
+ * with a dirty state.
+ */
+resettable_state_clear(&dev->reset);
+
QLIST_FOREACH(bus, &dev->child_bus, sibling) {
object_property_set_bool(OBJECT(bus), true, "realized",
&local_err);
@@ -945,7 +951,14 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
}
}
if (dev->hotplugged) {
-device_legacy_reset(dev);
+/*
+ * Reset the device, as well as its subtree which, at this point,
+ * should be realized too.
+ */
+resettable_assert_reset(OBJECT(dev), RESET_TYPE_COLD);
+resettable_change_parent(OBJECT(dev), OBJECT(dev->parent_bus),
+ NULL);
+resettable_release_reset(OBJECT(dev), RESET_TYPE_COLD);
}
dev->pending_deleted_event = false;
--
2.24.1
[PATCH v7 03/11] hw/core: create Resettable QOM interface
This commit defines an interface allowing multi-phase reset. This aims
to solve a problem of the actual single-phase reset (built in
DeviceClass and BusClass): reset behavior is dependent on the order
in which reset handlers are called. In particular doing external
side-effect (like setting an qemu_irq) is problematic because receiving
object may not be reset yet.
The Resettable interface divides the reset in 3 well defined phases.
To reset an object tree, all 1st phases are executed then all 2nd then
all 3rd. See the comments in include/hw/resettable.h for a more complete
description. The interface defines 3 phases to let the future
possibility of holding an object into reset for some time.
The qdev/qbus reset in DeviceClass and BusClass will be modified in
following commits to use this interface. A mechanism is provided
to allow executing a transitional reset handler in place of the 2nd
phase which is executed in children-then-parent order inside a tree.
This will allow to transition devices and buses smoothly while
keeping the exact current qdev/qbus reset behavior for now.
Documentation will be added in a following commit.
Signed-off-by: Damien Hedde
Reviewed-by: Richard Henderson
---
v7 update: un-nest struct ResettablePhases
---
Makefile.objs | 1 +
include/hw/resettable.h | 211 +++
hw/core/resettable.c| 238
hw/core/Makefile.objs | 1 +
hw/core/trace-events| 17 +++
5 files changed, 468 insertions(+)
create mode 100644 include/hw/resettable.h
create mode 100644 hw/core/resettable.c
diff --git a/Makefile.objs b/Makefile.objs
index 7c1e50f9d6..9752d549b4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -191,6 +191,7 @@ trace-events-subdirs += migration
trace-events-subdirs += net
trace-events-subdirs += ui
endif
+trace-events-subdirs += hw/core
trace-events-subdirs += hw/display
trace-events-subdirs += qapi
trace-events-subdirs += qom
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
new file mode 100644
index 00..58b3df4c22
--- /dev/null
+++ b/include/hw/resettable.h
@@ -0,0 +1,211 @@
+/*
+ * Resettable interface header.
+ *
+ * Copyright (c) 2019 GreenSocs SAS
+ *
+ * Authors:
+ * Damien Hedde
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_RESETTABLE_H
+#define HW_RESETTABLE_H
+
+#include "qom/object.h"
+
+#define TYPE_RESETTABLE_INTERFACE "resettable"
+
+#define RESETTABLE_CLASS(class) \
+OBJECT_CLASS_CHECK(ResettableClass, (class), TYPE_RESETTABLE_INTERFACE)
+
+#define RESETTABLE_GET_CLASS(obj) \
+OBJECT_GET_CLASS(ResettableClass, (obj), TYPE_RESETTABLE_INTERFACE)
+
+typedef struct ResettableState ResettableState;
+
+/**
+ * ResetType:
+ * Types of reset.
+ *
+ * + Cold: reset resulting from a power cycle of the object.
+ *
+ * TODO: Support has to be added to handle more types. In particular,
+ * ResettableState structure needs to be expanded.
+ */
+typedef enum ResetType {
+RESET_TYPE_COLD,
+} ResetType;
+
+/*
+ * ResettableClass:
+ * Interface for resettable objects.
+ *
+ * See docs/devel/reset.rst for more detailed information about how QEMU models
+ * reset. This whole API must only be used when holding the iothread mutex.
+ *
+ * All objects which can be reset must implement this interface;
+ * it is usually provided by a base class such as DeviceClass or BusClass.
+ * Every Resettable object must maintain some state tracking the
+ * progress of a reset operation by providing a ResettableState structure.
+ * The functions defined in this module take care of updating the
+ * state of the reset.
+ * The base class implementation of the interface provides this
+ * state and implements the associated method: get_state.
+ *
+ * Concrete object implementations (typically specific devices
+ * such as a UART model) should provide the functions
+ * for the phases.enter, phases.hold and phases.exit methods, which
+ * they can set in their class init function, either directly or
+ * by calling resettable_class_set_parent_phases().
+ * The phase methods are guaranteed to only only ever be called once
+ * for any reset event, in the order 'enter', 'hold', 'exit'.
+ * An object will always move quickly from 'enter' to 'hold'
+ * but might remain in 'hold' for an arbitrary period of time
+ * before eventually reset is deasserted and the 'exit' phase is called.
+ * Object implementations should be prepared for functions handling
+ * inbound connections from other devices (such as qemu_irq handler
+ * functions) to be called at any point during reset after their
+ * 'enter' method has been called.
+ *
+ * Users of a resettable object should not call these methods
+ * directly, but instead use the function resettable_reset().
+ *
+ * @phases.enter: This phase is called when the object enters reset. It
+ * should reset local state of the object, but it mu
[PATCH v7 05/11] hw/core/resettable: add support for changing parent
Add a function resettable_change_parent() to do the required
plumbing when changing the parent a of Resettable object.
We need to make sure that the reset state of the object remains
coherent with the reset state of the new parent.
We make the 2 following hypothesis:
+ when an object is put in a parent under reset, the object goes in
reset.
+ when an object is removed from a parent under reset, the object
leaves reset.
The added function avoids any glitch if both old and new parent are
already in reset.
Signed-off-by: Damien Hedde
Reviewed-by: Richard Henderson
---
include/hw/resettable.h | 16 +++
hw/core/resettable.c| 62 +++--
hw/core/trace-events| 1 +
3 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
index 58b3df4c22..3c87ab86c7 100644
--- a/include/hw/resettable.h
+++ b/include/hw/resettable.h
@@ -194,6 +194,22 @@ void resettable_release_reset(Object *obj, ResetType type);
*/
bool resettable_is_in_reset(Object *obj);
+/**
+ * resettable_change_parent:
+ * Indicate that the parent of Ressettable @obj is changing from @oldp to
@newp.
+ * All 3 objects must implement resettable interface. @oldp or @newp may be
+ * NULL.
+ *
+ * This function will adapt the reset state of @obj so that it is coherent
+ * with the reset state of @newp. It may trigger @resettable_assert_reset()
+ * or @resettable_release_reset(). It will do such things only if the reset
+ * state of @newp and @oldp are different.
+ *
+ * When using this function during reset, it must only be called during
+ * a hold phase method. Calling this during enter or exit phase is an error.
+ */
+void resettable_change_parent(Object *obj, Object *newp, Object *oldp);
+
/**
* resettable_class_set_parent_phases:
*
diff --git a/hw/core/resettable.c b/hw/core/resettable.c
index 9133208487..e99bb30058 100644
--- a/hw/core/resettable.c
+++ b/hw/core/resettable.c
@@ -28,12 +28,16 @@ static void resettable_phase_exit(Object *obj, void
*opaque, ResetType type);
* enter_phase_in_progress:
* True if we are currently in reset enter phase.
*
- * Note: This flag is only used to guarantee (using asserts) that the reset
- * API is used correctly. We can use a global variable because we rely on the
+ * exit_phase_in_progress:
+ * count the number of exit phase we are in.
+ *
+ * Note: These flags are only used to guarantee (using asserts) that the reset
+ * API is used correctly. We can use global variables because we rely on the
* iothread mutex to ensure only one reset operation is in a progress at a
* given time.
*/
static bool enter_phase_in_progress;
+static unsigned exit_phase_in_progress;
void resettable_reset(Object *obj, ResetType type)
{
@@ -65,7 +69,9 @@ void resettable_release_reset(Object *obj, ResetType type)
trace_resettable_reset_release_begin(obj, type);
assert(!enter_phase_in_progress);
+exit_phase_in_progress += 1;
resettable_phase_exit(obj, NULL, type);
+exit_phase_in_progress -= 1;
trace_resettable_reset_release_end(obj);
}
@@ -206,6 +212,58 @@ static void resettable_phase_exit(Object *obj, void
*opaque, ResetType type)
trace_resettable_phase_exit_end(obj, obj_typename, s->count);
}
+/*
+ * resettable_get_count:
+ * Get the count of the Resettable object @obj. Return 0 if @obj is NULL.
+ */
+static uint32_t resettable_get_count(Object *obj)
+{
+if (obj) {
+ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+return rc->get_state(obj)->count;
+}
+return 0;
+}
+
+void resettable_change_parent(Object *obj, Object *newp, Object *oldp)
+{
+ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+ResettableState *s = rc->get_state(obj);
+uint32_t newp_count = resettable_get_count(newp);
+uint32_t oldp_count = resettable_get_count(oldp);
+
+/*
+ * Ensure we do not change parent when in enter or exit phase.
+ * During these phases, the reset subtree being updated is partly in reset
+ * and partly not in reset (it depends on the actual position in
+ * resettable_child_foreach()s). We are not able to tell in which part is a
+ * leaving or arriving device. Thus we cannot set the reset count of the
+ * moving device to the proper value.
+ */
+assert(!enter_phase_in_progress && !exit_phase_in_progress);
+trace_resettable_change_parent(obj, oldp, oldp_count, newp, newp_count);
+
+/*
+ * At most one of the two 'for' loops will be executed below
+ * in order to cope with the difference between the two counts.
+ */
+/* if newp is more reset than oldp */
+for (uint32_t i = oldp_count; i < newp_count; i++) {
+resettable_assert_reset(obj, RESET_TYPE_COLD);
+}
+/*
+ * if obj is leaving a bus under reset, we need to ensure
+ * hold phase is not pending.
+ */
+if (oldp_count && s->hold_phase_pending) {
+resettable_phase_hold(obj, NULL, RESET
[PATCH v7 10/11] vl: replace deprecated qbus_reset_all registration
Replace deprecated qbus_reset_all by resettable_cold_reset_fn for
the sysbus reset registration.
Apart for the raspi machines, this does not impact the behavior
because:
+ at this point resettable just calls the old reset methods of devices
and buses in the same order as qdev/qbus.
+ resettable handlers registered with qemu_register_reset are
serialized; there is no interleaving.
+ eventual explicit calls to legacy reset API (device_reset or
qdev/qbus_reset) inside this reset handler will not be masked out
by resettable mechanism; they do not go through resettable api.
For the raspi machines, during the sysbus reset the sd-card is not
reset twice anymore but only once. This is a consequence of switching
both sysbus reset and changing parent to resettable; it detects the
second reset is not needed. This has no impact on the state after
reset; the sd-card reset method only reset local state and query
information from the block backend.
Signed-off-by: Damien Hedde
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
The raspi reset change can be observed by using the following command
(reset will occurs, then do Ctrl-C to end qemu; no firmware is
given here).
qemu-system-aarch64 -M raspi3 \
-trace resettable_phase_hold_exec \
-trace qdev_update_parent_bus \
-trace resettable_change_parent \
-trace qdev_reset -trace qbus_reset
Before the patch, the qdev/qbus_reset traces show when reset method are
called. After the patch, the resettable_phase_hold_exec show when reset
method are called.
The traced reset order of the raspi3 is listed below. I've added empty
lines and the tree structure.
+->bcm2835-peripherals reset
|
| +->sd-card reset
| +->sd-bus reset
+->bcm2835_gpio reset
| -> dev_update_parent_bus (move the sd-card on the sdhci-bus)
| -> resettable_change_parent
|
+->bcm2835-dma reset
|
| +->bcm2835-sdhost-bus reset
+->bcm2835-sdhost reset
|
| +->sd-card (reset ONLY BEFORE BEFORE THE PATCH)
| +->sdhci-bus reset
+->generic-sdhci reset
|
+->bcm2835-rng reset
+->bcm2835-property reset
+->bcm2835-fb reset
+->bcm2835-mbox reset
+->bcm2835-aux reset
+->pl011 reset
+->bcm2835-ic reset
+->bcm2836-control reset
System reset
In both case, the sd-card is reset (being on bcm2835_gpio/sd-bus) then moved
to generic-sdhci/sdhci-bus by the bcm2835_gpio reset method.
Before the patch, it is then reset again being part of generic-sdhci/sdhci-bus.
After the patch, it considered again for reset but its reset method is not
called because it is already flagged as reset.
---
vl.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 751401214c..e5a537d4f9 100644
--- a/vl.c
+++ b/vl.c
@@ -4362,7 +4362,15 @@ int main(int argc, char **argv, char **envp)
/* TODO: once all bus devices are qdevified, this should be done
* when bus is created by qdev.c */
-qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());
+/*
+ * TODO: If we had a main 'reset container' that the whole system
+ * lived in, we could reset that using the multi-phase reset
+ * APIs. For the moment, we just reset the sysbus, which will cause
+ * all devices hanging off it (and all their child buses, recursively)
+ * to be reset. Note that this will *not* reset any Device objects
+ * which are not attached to some part of the qbus tree!
+ */
+qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default());
qemu_run_machine_init_done_notifiers();
if (rom_check_and_register_reset() != 0) {
--
2.24.1
[PATCH v7 06/11] hw/core/qdev: handle parent bus change regarding resettable
In qdev_set_parent_bus(), when changing the parent bus of a
realized device, if the source and destination buses are not in the
same reset state, some adaptations are required. This patch adds
needed call to resettable_change_parent() to make sure a device reset
state stays coherent with its parent bus.
The addition is a no-op if:
1. the device being parented is not realized.
2. the device is realized, but both buses are not under reset.
Case 2 means that as long as qdev_set_parent_bus() is called
during the machine realization procedure (which is before the
machine reset so nothing is in reset), it is a no op.
There are 52 call sites of qdev_set_parent_bus(). All but one fall
into the no-op case:
+ 29 trivial calls related to virtio (in hw/{s390x,display,virtio}/
{vhost,virtio}-xxx.c) to set a vdev(or vgpu) composing device
parent bus just before realizing the same vdev(vgpu).
+ hw/core/qdev.c: when creating a device in qdev_try_create()
+ hw/core/sysbus.c: when initializing a device in the sysbus
+ hw/i386/amd_iommu.c: before realizing AMDVIState/pci
+ hw/isa/piix4.c: before realizing PIIX4State/rtc
+ hw/misc/auxbus.c: when creating an AUXBus
+ hw/misc/auxbus.c: when creating an AUXBus child
+ hw/misc/macio/macio.c: when initializing a MACIOState child
+ hw/misc/macio/macio.c: before realizing NewWorldMacIOState/pmu
+ hw/misc/macio/macio.c: before realizing NewWorldMacIOState/cuda
+ hw/net/virtio-net.c: Used for migration when using the failover
mechanism to migration a vfio-pci/net. It is
a no-op because at this point the device is
already on the bus.
+ hw/pci-host/designware.c: before realizing DesignwarePCIEHost/root
+ hw/pci-host/gpex.c: before realizing GPEXHost/root
+ hw/pci-host/prep.c: when initializaing PREPPCIState/pci_dev
+ hw/pci-host/q35.c: before realizing Q35PCIHost/mch
+ hw/pci-host/versatile.c: when initializing PCIVPBState/pci_dev
+ hw/pci-host/xilinx-pcie.c: before realizing XilinxPCIEHost/root
+ hw/s390x/event-facility.c: when creating SCLPEventFacility/
TYPE_SCLP_QUIESCE
+ hw/s390x/event-facility.c: ditto with SCLPEventFacility/
TYPE_SCLP_CPU_HOTPLUG
+ hw/s390x/sclp.c: Not trivial because it is called on a SLCPDevice
just after realizing it. Ok because at this point the destination
bus (sysbus) is not in reset; the realize step is before the
machine reset.
+ hw/sd/core.c: Not OK. Used in sdbus_reparent_card(). See below.
+ hw/ssi/ssi.c: Used to put spi slave on spi bus and connect the cs
line in ssi_auto_connect_slave(). Ok because this function is only
used in realize step in hw/ssi/aspeed_smc.ci, hw/ssi/imx_spi.c,
hw/ssi/mss-spi.c, hw/ssi/xilinx_spi.c and hw/ssi/xilinx_spips.c.
+ hw/xen/xen-legacy-backend.c: when creating a XenLegacyDevice device
+ qdev-monitor.c: in device hotplug creation procedure before realize
Note that this commit alone will have no effect, right now there is no
use of resettable API to reset anything. So a bus will never be tagged
as in-reset by this same API.
The one place where side-effect will occurs is in hw/sd/core.c in
sdbus_reparent_card(). This function is only used in the raspi machines,
including during the sysbus reset procedure. This case will be
carrefully handled when doing the multiple phase reset transition.
Signed-off-by: Damien Hedde
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
v6 update: there are now 3 more call sites (52 instead of 49).
+ hw/isa/piix4.c
+ hw/net/virtio-net.c
+ hw/virtio/vhost-user-fs-pci.c (in list below)
Exhaustive list of the 29 "virtio" caller to qdev_set_parent_bus():
+ hw/display/virtio-gpu-pci.c: VirtIOGPUPCIBase/vgpu realize
+ hw/display/virtio-vga.c: VirtIOVGABase/vgpu realize
+ hw/s390x/vhost-vsock-ccw.c: VHostVSockCCWState/vdev realize
+ hw/s390x/virtio-ccw-9p.c:V9fsCCWState/vdev realize
+ hw/s390x/virtio-ccw-balloon.c: VirtIOBalloonCcw/vdev realize
+ hw/s390x/virtio-ccw-blk.c: VirtIOBlkCcw/vdev realize
+ hw/s390x/virtio-ccw-crypto.c:VirtIOCryptoCcw/vdev realize
+ hw/s390x/virtio-ccw-gpu.c: VirtIOGPUCcw/vdev realize
+ hw/s390x/virtio-ccw-input.c: VirtIOInputCcw/vdev realize
+ hw/s390x/virtio-ccw-net.c: VirtIONetCcw/vdev realize
+ hw/s390x/virtio-ccw-rng.c: VirtIORNGCcw/vdev realize
+ hw/s390x/virtio-ccw-scsi.c: VirtIOSCSICcw/vdev realize
+ hw/s390x/virtio-ccw-scsi.c: VHostSCSICcw/vdev realize
+ hw/s390x/virtio-ccw-serial.c:VirtioSerialCcw/vdev realize
+ hw/virtio/vhost-scsi-pci.c: VHostSCSIPCI/vdev realize
+ hw/virtio/vhost-user-blk-pci.c: VHostUserBlkPCI/vdev realize
+ hw/virtio/vhost-user-fs-pci.c: VHostUserFSPCI/vdev realize
+ hw/virtio/vhost-user-scsi-pci.c: VHostUserSCSIPCI/vdev realize
+ hw/virtio/vhost-vsock-pci.c: VHostVSockPCI/vdev realize
+ hw/virtio/virtio-9p-pci.c: V9fsPCIState/vdev realize
+ hw/virtio/virtio-balloon-pci.c: VirtIOBalloonPCI/
[PATCH v7 11/11] hw/s390x/ipl: replace deprecated qdev_reset_all registration
Replace deprecated qdev_reset_all by resettable_cold_reset_fn for the ipl registration in the main reset handlers. This does not impact the behavior for the following reasons: + at this point resettable just call the old reset methods of devices and buses in the same order than qdev/qbus. + resettable handlers registered with qemu_register_reset are serialized; there is no interleaving. + eventual explicit calls to legacy reset API (device_reset or qdev/qbus_reset) inside this reset handler will not be masked out by resettable mechanism; they do not go through resettable api. Signed-off-by: Damien Hedde Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Reviewed-by: Cornelia Huck Reviewed-by: Richard Henderson --- Cc: qemu-s3...@nongnu.org Cc: Christian Borntraeger Cc: Thomas Huth --- hw/s390x/ipl.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index ca544d64c5..2689f7a017 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -237,7 +237,15 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) */ ipl->compat_start_addr = ipl->start_addr; ipl->compat_bios_start_addr = ipl->bios_start_addr; -qemu_register_reset(qdev_reset_all_fn, dev); +/* + * Because this Device is not on any bus in the qbus tree (it is + * not a sysbus device and it's not on some other bus like a PCI + * bus) it will not be automatically reset by the 'reset the + * sysbus' hook registered by vl.c like most devices. So we must + * manually register a reset hook for it. + * TODO: there should be a better way to do this. + */ +qemu_register_reset(resettable_cold_reset_fn, dev); error: error_propagate(errp, err); } -- 2.24.1
[PATCH v7 08/11] hw/core: deprecate old reset functions and introduce new ones
Deprecate device_legacy_reset(), qdev_reset_all() and
qbus_reset_all() to be replaced by new functions
device_cold_reset() and bus_cold_reset() which uses resettable API.
Also introduce resettable_cold_reset_fn() which may be used as a
replacement for qdev_reset_all_fn and qbus_reset_all_fn().
Following patches will be needed to look at legacy reset call sites
and switch to resettable api. The legacy functions will be removed
when unused.
Signed-off-by: Damien Hedde
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
include/hw/qdev-core.h | 27 +++
include/hw/resettable.h | 9 +
hw/core/bus.c | 5 +
hw/core/qdev.c | 5 +
hw/core/resettable.c| 5 +
5 files changed, 51 insertions(+)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1b4b420617..b84fcc32bf 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -406,6 +406,13 @@ int qdev_walk_children(DeviceState *dev,
qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
void *opaque);
+/**
+ * @qdev_reset_all:
+ * Reset @dev. See @qbus_reset_all() for more details.
+ *
+ * Note: This function is deprecated and will be removed when it becomes
unused.
+ * Please use device_cold_reset() now.
+ */
void qdev_reset_all(DeviceState *dev);
void qdev_reset_all_fn(void *opaque);
@@ -418,10 +425,28 @@ void qdev_reset_all_fn(void *opaque);
* hard reset means that qbus_reset_all will reset all state of the device.
* For PCI devices, for example, this will include the base address registers
* or configuration space.
+ *
+ * Note: This function is deprecated and will be removed when it becomes
unused.
+ * Please use bus_cold_reset() now.
*/
void qbus_reset_all(BusState *bus);
void qbus_reset_all_fn(void *opaque);
+/**
+ * device_cold_reset:
+ * Reset device @dev and perform a recursive processing using the resettable
+ * interface. It triggers a RESET_TYPE_COLD.
+ */
+void device_cold_reset(DeviceState *dev);
+
+/**
+ * bus_cold_reset:
+ *
+ * Reset bus @bus and perform a recursive processing using the resettable
+ * interface. It triggers a RESET_TYPE_COLD.
+ */
+void bus_cold_reset(BusState *bus);
+
/**
* device_is_in_reset:
* Return true if the device @dev is currently being reset.
@@ -452,6 +477,8 @@ void qdev_machine_init(void);
* device_legacy_reset:
*
* Reset a single device (by calling the reset method).
+ * Note: This function is deprecated and will be removed when it becomes
unused.
+ * Please use device_cold_reset() now.
*/
void device_legacy_reset(DeviceState *dev);
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
index 3f90da5b5b..0c1adf49ee 100644
--- a/include/hw/resettable.h
+++ b/include/hw/resettable.h
@@ -221,6 +221,15 @@ bool resettable_is_in_reset(Object *obj);
*/
void resettable_change_parent(Object *obj, Object *newp, Object *oldp);
+/**
+ * resettable_cold_reset_fn:
+ * Helper to call resettable_reset((Object *) opaque, RESET_TYPE_COLD).
+ *
+ * This function is typically useful to register a reset handler with
+ * qemu_register_reset.
+ */
+void resettable_cold_reset_fn(void *opaque);
+
/**
* resettable_class_set_parent_phases:
*
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 2698f715bd..3dc0a825f0 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -68,6 +68,11 @@ int qbus_walk_children(BusState *bus,
return 0;
}
+void bus_cold_reset(BusState *bus)
+{
+resettable_reset(OBJECT(bus), RESET_TYPE_COLD);
+}
+
bool bus_is_in_reset(BusState *bus)
{
return resettable_is_in_reset(OBJECT(bus));
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 5fac4cd8fc..a7a7abe569 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -361,6 +361,11 @@ void qbus_reset_all_fn(void *opaque)
qbus_reset_all(bus);
}
+void device_cold_reset(DeviceState *dev)
+{
+resettable_reset(OBJECT(dev), RESET_TYPE_COLD);
+}
+
bool device_is_in_reset(DeviceState *dev)
{
return resettable_is_in_reset(OBJECT(dev));
diff --git a/hw/core/resettable.c b/hw/core/resettable.c
index e99bb30058..7647ef6863 100644
--- a/hw/core/resettable.c
+++ b/hw/core/resettable.c
@@ -264,6 +264,11 @@ void resettable_change_parent(Object *obj, Object *newp,
Object *oldp)
}
}
+void resettable_cold_reset_fn(void *opaque)
+{
+resettable_reset((Object *) opaque, RESET_TYPE_COLD);
+}
+
void resettable_class_set_parent_phases(ResettableClass *rc,
ResettableEnterPhase enter,
ResettableHoldPhase hold,
--
2.24.1
[PATCH v7 09/11] docs/devel/reset.rst: add doc about Resettable interface
Signed-off-by: Damien Hedde Reviewed-by: Peter Maydell Reviewed-by: Richard Henderson --- docs/devel/index.rst | 1 + docs/devel/reset.rst | 289 +++ 2 files changed, 290 insertions(+) create mode 100644 docs/devel/reset.rst diff --git a/docs/devel/index.rst b/docs/devel/index.rst index ac862152dc..4dc2ca8d71 100644 --- a/docs/devel/index.rst +++ b/docs/devel/index.rst @@ -24,3 +24,4 @@ Contents: tcg tcg-plugins bitops + reset diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst new file mode 100644 index 00..abea1102dc --- /dev/null +++ b/docs/devel/reset.rst @@ -0,0 +1,289 @@ + +=== +Reset in QEMU: the Resettable interface +=== + +The reset of qemu objects is handled using the resettable interface declared +in ``include/hw/resettable.h``. + +This interface allows objects to be grouped (on a tree basis); so that the +whole group can be reset consistently. Each individual member object does not +have to care about others; in particular, problems of order (which object is +reset first) are addressed. + +As of now DeviceClass and BusClass implement this interface. + + +Triggering reset + + +This section documents the APIs which "users" of a resettable object should use +to control it. All resettable control functions must be called while holding +the iothread lock. + +You can apply a reset to an object using ``resettable_assert_reset()``. You need +to call ``resettable_release_reset()`` to release the object from reset. To +instantly reset an object, without keeping it in reset state, just call +``resettable_reset()``. These functions take two parameters: a pointer to the +object to reset and a reset type. + +Several types of reset will be supported. For now only cold reset is defined; +others may be added later. The Resettable interface handles reset types with an +enum: + +``RESET_TYPE_COLD`` + Cold reset is supported by every resettable object. In QEMU, it means we reset + to the initial state corresponding to the start of QEMU; this might differ + from what is a real hardware cold reset. It differs from other resets (like + warm or bus resets) which may keep certain parts untouched. + +Calling ``resettable_reset()`` is equivalent to calling +``resettable_assert_reset()`` then ``resettable_release_reset()``. It is +possible to interleave multiple calls to these three functions. There may +be several reset sources/controllers of a given object. The interface handles +everything and the different reset controllers do not need to know anything +about each others. The object will leave reset state only when each other +controllers end their reset operation. This point is handled internally by +maintaining a count of in-progress resets; it is crucial to call +``resettable_release_reset()`` one time and only one time per +``resettable_assert_reset()`` call. + +For now migration of a device or bus in reset is not supported. Care must be +taken not to delay ``resettable_release_reset()`` after its +``resettable_assert_reset()`` counterpart. + +Note that, since resettable is an interface, the API takes a simple Object as +parameter. Still, it is a programming error to call a resettable function on a +non-resettable object and it will trigger a run time assert error. Since most +calls to resettable interface are done through base class functions, such an +error is not likely to happen. + +For Devices and Buses, the following helper functions exist: + +- ``device_cold_reset()`` +- ``bus_cold_reset()`` + +These are simple wrappers around resettable_reset() function; they only cast the +Device or Bus into an Object and pass the cold reset type. When possible +prefer to use these functions instead of ``resettable_reset()``. + +Device and bus functions co-exist because there can be semantic differences +between resetting a bus and resetting the controller bridge which owns it. +For example, consider a SCSI controller. Resetting the controller puts all +its registers back to what reset state was as well as reset everything on the +SCSI bus, whereas resetting just the SCSI bus only resets everything that's on +it but not the controller. + + +Multi-phase mechanism +- + +This section documents the internals of the resettable interface. + +The resettable interface uses a multi-phase system to relieve objects and +machines from reset ordering problems. To address this, the reset operation +of an object is split into three well defined phases. + +When resetting several objects (for example the whole machine at simulation +startup), all first phases of all objects are executed, then all second phases +and then all third phases. + +The three phases are: + +1. The **enter** phase is executed when the object enters reset. It resets only + local state of the object; it must not do anything that has a side-effect + on other objects, such as ra
Re: [PATCH v12 05/13] virtio-iommu: Endpoint and domains structs and helpers
Hi Peter,
On 1/14/20 7:07 PM, Peter Xu wrote:
> On Tue, Jan 14, 2020 at 09:51:59AM +0100, Auger Eric wrote:
>> Hi Peter,
>
> Hi, Eric,
>
> [...]
>
>>>
+{
+VirtIOIOMMUEndpoint *ep;
+
+ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+if (ep) {
+return ep;
+}
+if (!virtio_iommu_mr(s, ep_id)) {
>>>
>>> Could I ask when this will trigger?
>>
>> This can happen when a device is attached to a domain and its RID does
>> not correspond to one of the devices protected by the iommu.
>
> So will it happen only because of a kernel driver bug?
Yes, at the moment, because virtio_iommu_mr() only gets called on device
attach to a domain.
The spec says:
"If the endpoint identified by endpoint doesn’t exist, the device MUST
reject the request and set status to VIRTIO_IOMMU_S_NOENT"
>
> Also, I think the name of "virtio_iommu_mr" is confusing on that it
> returned explicitly a MemoryRegion however it's never been used:
I use the same prototype as for smmu_iommu_mr(). Returning the iommu mr
will allow to proceed with further RID based operations like invalidations.
The same logic is used in vtd_context_device_invalidate.
>
> (since they're not in the same patch I'm pasting)
>
> static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
> {
> uint8_t bus_n, devfn;
> IOMMUPciBus *iommu_pci_bus;
> IOMMUDevice *dev;
>
> bus_n = PCI_BUS_NUM(sid);
> iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
> if (iommu_pci_bus) {
> devfn = sid & 0xFF;
> dev = iommu_pci_bus->pbdev[devfn];
> if (dev) {
> return &dev->iommu_mr;
> }
> }
> return NULL;
> }
>
> Maybe "return !!dev" would be enough, then make the return a boolean?
> Then we can rename it to virtio_iommu_has_device().
>
> PS. I think we can also drop IOMMU_PCI_DEVFN_MAX (after all you even
> didn't use it here!) and use PCI_DEVFN_MAX, and replace 0xFF.
well intel iommu and smmu use a similar constant (PCI_DEVFN_MAX,
SMMU_PCI_DEVFN_MAX resp.). I use it in virtio_iommu_find_add_as
Thanks
Eric
>
> Thanks,
>
RE: [PATCH v22 0/9] Add ARMv8 RAS virtualization support in QEMU
Ping. 发件人：gengdongjiu 收件人：pbonzini ;gengdongjiu ;mst ;imammedo ;shannon.zhaosl ;peter.maydell ;fam ;rth ;ehabkost ;mtosatti ;xuwei (O) ;Jonathan Cameron ;james.morse ;qemu-devel ;kvm ;qemu-arm 抄 送：zhengxiang (A) ;Linuxarm 时 间：2020-01-08 19:32:33 主题[PATCH v22 0/9] Add ARMv8 RAS virtualization support in QEMU In the ARMv8 platform, the CPU error types are synchronous external abort(SEA) and SError Interrupt (SEI). If exception happens in guest, sometimes it's better for guest to perform the recovery, because host does not know the detailed information of guest. For example, if an exception happens in a user-space application within guest, host does not know which application encounters errors. For the ARMv8 SEA/SEI, KVM or host kernel delivers SIGBUS to notify userspace. After user space gets the notification, it will record the CPER into guest GHES buffer and inject an exception or IRQ into guest. In the current implementation, if the type of SIGBUS is BUS_MCEERR_AR, we will treat it as a synchronous exception, and notify guest with ARMv8 SEA notification type after recording CPER into guest. This series of patches are based on Qemu 4.2, which include two parts: 1. Generate APEI/GHES table. 2. Handle the SIGBUS signal, record the CPER in runtime and fill it into guest memory, then notify guest according to the type of SIGBUS. The whole solution was suggested by James(james.mo...@arm.com); The solution of APEI section was suggested by Laszlo(ler...@redhat.com). Show some discussions in [1]. This series of patches have already been tested on ARM64 platform with RAS feature enabled: Show the APEI part verification result in [2]. Show the BUS_MCEERR_AR SIGBUS handling verification result in [3]. --- Change since v21: 1. Make the user-facing 'ras' option description more clearly to address Peter's comments. 2. Update the doc description in "docs/specs/acpi_hest_ghes.rst" 3. Split HEST/GHES patches to more patches to make the review easily 4. Using source_id to index the location to save the CPER. 5. Optimize and simplify the logic to build HEST/GHES table to address Igor/Michael/Beata comments. 6. make ghes_addr_le a part of GED device. Change since v20: 1. Move some implementation details from acpi_ghes.h to acpi_ghes.c 2. Add the reviewers for the ACPI/APEI/GHES part Change since v19: 1. Fix clang compile error 2. Fix sphinx build error Change since v18: 1. Fix some code-style and typo/grammar problems. 2. Remove no_ras in the VirtMachineClass struct. 3. Convert documentation to rst format. 4. Simplize the code and add comments for some magic value. 5. Move kvm_inject_arm_sea() function into the patch where it's used. 6. Register the reset handler(kvm_unpoison_all()) in the kvm_init() function. Change since v17: 1. Improve some commit messages and comments. 2. Fix some code-style problems. 3. Add a *ras* machine option. 4. Move HEST/GHES related structures and macros into "hw/acpi/acpi_ghes.*". 5. Move HWPoison page functions into "include/sysemu/kvm_int.h". 6. Fix some bugs. 7. Improve the design document. Change since v16: 1. check whether ACPI table is enabled when handling the memory error in the SIGBUS handler. Change since v15: 1. Add a doc-comment in the proper format for 'include/exec/ram_addr.h' 2. Remove write_part_cpustate_to_list() because there is another bug fix patch has been merged "arm: Allow system registers for KVM guests to be changed by QEMU code" 3. Add some comments for kvm_inject_arm_sea() in 'target/arm/kvm64.c' 4. Compare the arm_current_el() return value to 0,1,2,3, not to PSTATE_MODE_* constants. 5. Change the RAS support wasn't introduced before 4.1 QEMU version. 6. Move the no_ras flag patch to begin in this series Change since v14: 1. Remove the BUS_MCEERR_AO handling logic because this asynchronous signal was masked by main thread 2. Address some Igor Mammedov's comments(ACPI part) 1) change the comments for the enum AcpiHestNotifyType definition and remove ditto in patch 1 2) change some patch commit messages and separate "APEI GHES table generation" patch to more patches. 3. Address some peter's comments(arm64 Synchronous External Abort injection) 1) change some code notes 2) using arm_current_el() for current EL 2) use the helper functions for those (syn_data_abort_*). Change since v13: 1. Move the patches that set guest ESR and inject virtual SError out of this series 2. Clean and optimize the APEI part patches 3. Update the commit messages and add some comments for the code Change since v12: 1. Address Paolo's comments to move HWPoisonPage definition to accel/kvm/kvm-all.c 2. Only call kvm_cpu_synchronize_state() when get the BUS_MCEERR_AR signal 3. Only add and enable GPIO-Signal and ARMv8 SEA two hardware error sources 4. Address Michael's comments to not sync SPDX from Linux kernel header file Change since v11: Address James's comments(james.mo...@arm.com) 1. Check whether KVM has the capability to to set
Re: [PATCH] hw/input: Do not enable CONFIG_PCKBD by default
On 15/01/20 12:37, Thomas Huth wrote: > The i8042 device is part of the chipset of a machine, so it should > be selected by the machines or chipsets (e.g. SuperIO chipsets), > and not be enabled by default. > > Signed-off-by: Thomas Huth > --- > hw/input/Kconfig | 1 - > hw/isa/Kconfig | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/input/Kconfig b/hw/input/Kconfig > index 287f08887b..a2f25725be 100644 > --- a/hw/input/Kconfig > +++ b/hw/input/Kconfig > @@ -7,7 +7,6 @@ config LM832X > > config PCKBD > bool > -default y > select PS2 > depends on ISA_BUS > > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > index 8a38813cc1..c7f07854f7 100644 > --- a/hw/isa/Kconfig > +++ b/hw/isa/Kconfig > @@ -16,6 +16,7 @@ config I82378 > config ISA_SUPERIO > bool > select ISA_BUS > +select PCKBD > > config PC87312 > bool > Since you CCed qemu-trivial, Acked-by: Paolo Bonzini
Re: [PATCH v2 0/4] qmp: Optionally run handlers in coroutines
On Tue, Jan 14, 2020 at 07:27:31PM +0100, Kevin Wolf wrote: > Some QMP command handlers can block the main loop for a relatively long > time, for example because they perform some I/O. This is quite nasty. > Allowing such handlers to run in a coroutine where they can yield (and > therefore release the BQL) while waiting for an event such as I/O > completion solves the problem. > > This series adds the infrastructure to allow this and switches > block_resize to run in a coroutine as a first example. > > This is an alternative solution to Marc-André's "monitor: add > asynchronous command type" series. > > v2: > - Fix typo in a commit message [Eric] > - Use hyphen instead of underscore for the test command [Eric] > - Mark qmp_block_resize() as coroutine_fn [Stefan] > > > Kevin Wolf (4): > qapi: Add a 'coroutine' flag for commands > vl: Initialise main loop earlier > qmp: Move dispatcher to a coroutine > block: Mark 'block_resize' as coroutine > > qapi/block-core.json| 3 +- > tests/qapi-schema/qapi-schema-test.json | 1 + > docs/devel/qapi-code-gen.txt| 4 ++ > include/qapi/qmp/dispatch.h | 3 + > monitor/monitor-internal.h | 5 +- > blockdev.c | 6 +- > monitor/monitor.c | 24 --- > monitor/qmp.c | 83 - > qapi/qmp-dispatch.c | 38 ++- > tests/test-qmp-cmds.c | 4 ++ > vl.c| 10 +-- > scripts/qapi/commands.py| 17 +++-- > scripts/qapi/doc.py | 2 +- > scripts/qapi/expr.py| 4 +- > scripts/qapi/introspect.py | 2 +- > scripts/qapi/schema.py | 9 ++- > tests/qapi-schema/qapi-schema-test.out | 2 + > tests/qapi-schema/test-qapi.py | 7 ++- > 18 files changed, 158 insertions(+), 66 deletions(-) > > -- > 2.20.1 > Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH v12 09/13] virtio-iommu: Implement fault reporting
Hi Peter,
On 1/14/20 8:04 PM, Peter Xu wrote:
> On Thu, Jan 09, 2020 at 03:43:15PM +0100, Eric Auger wrote:
>> The event queue allows to report asynchronous errors.
>> The translate function now injects faults when relevant.
>>
>> Signed-off-by: Eric Auger
>>
>> ---
>>
>> v11 -> v12:
>> - reporting the addr associated with the fault and set the
>> VIRTIO_IOMMU_FAULT_F_ADDRESS flag.
>> - added cpu_to_le
>>
>> v10 -> v11:
>> - change a virtio_error into an error_report_once
>> (no buffer available for output faults)
>> ---
>> hw/virtio/trace-events | 1 +
>> hw/virtio/virtio-iommu.c | 73 +---
>> 2 files changed, 69 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>> index 095aa8b509..e83500bee9 100644
>> --- a/hw/virtio/trace-events
>> +++ b/hw/virtio/trace-events
>> @@ -72,3 +72,4 @@ virtio_iommu_put_endpoint(uint32_t ep_id) "Free
>> endpoint=%d"
>> virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
>> virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
>> virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t
>> sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
>> +virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t
>> endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address
>> =0x%"PRIx64
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index d192bcb505..09193970ee 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -477,6 +477,51 @@ out:
>> }
>> }
>>
>> +static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
>> + int flags, uint32_t endpoint,
>> + uint64_t address)
>> +{
>> +VirtIODevice *vdev = &viommu->parent_obj;
>> +VirtQueue *vq = viommu->event_vq;
>> +struct virtio_iommu_fault fault;
>> +VirtQueueElement *elem;
>> +size_t sz;
>> +
>> +memset(&fault, 0, sizeof(fault));
>> +fault.reason = reason;
>> +fault.flags = cpu_to_le32(flags);
>> +fault.endpoint = cpu_to_le32(endpoint);
>> +fault.address = cpu_to_le64(address);
>> +
>> +for (;;) {
>> +elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>> +
>> +if (!elem) {
>> +error_report_once(
>> +"no buffer available in event queue to report event");
>
> (Should this also be a guest issue? IIRC you are still using a
> mixture of both qemu_log_mask and error_report()... I'll stop
> commenting on this, assuming that you prefer both ways to be used...)
I've just removed the qemu_log_mask in virtio_iommu_unmap(). So now you
should not find any qemu_log_mask anymore. Sorry for the oversight.
>
>> +return;
>> +}
>> +
>> +if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
>> +virtio_error(vdev, "error buffer of wrong size");
>> +virtqueue_detach_element(vq, elem, 0);
>> +g_free(elem);
>> +continue;
>
> If virtio_error(), should we stop rather than continue?
My understanding is the buffer just popped had a wrong size so it is not
usable. We skip it we try to use another one if any. Does it make sense?
Thanks
Eric
>
>> +}
>> +break;
>> +}
>> +/* we have a buffer to fill in */
>> +sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
>> + &fault, sizeof(fault));
>> +assert(sz == sizeof(fault));
>> +
>> +trace_virtio_iommu_report_fault(reason, flags, endpoint, address);
>> +virtqueue_push(vq, elem, sz);
>> +virtio_notify(vdev, vq);
>> +g_free(elem);
>> +
>> +}
>> +
>> static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr
>> addr,
>> IOMMUAccessFlags flag,
>> int iommu_idx)
>> @@ -485,9 +530,10 @@ static IOMMUTLBEntry
>> virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>> VirtIOIOMMUInterval interval, *mapping_key;
>> VirtIOIOMMUMapping *mapping_value;
>> VirtIOIOMMU *s = sdev->viommu;
>> +bool read_fault, write_fault;
>> VirtIOIOMMUEndpoint *ep;
>> +uint32_t sid, flags;
>> bool bypass_allowed;
>> -uint32_t sid;
>> bool found;
>>
>> interval.low = addr;
>> @@ -513,6 +559,9 @@ static IOMMUTLBEntry
>> virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>> if (!ep) {
>> if (!bypass_allowed) {
>> error_report_once("%s sid=%d is not known!!", __func__, sid);
>> +virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_UNKNOWN,
>> + VIRTIO_IOMMU_FAULT_F_ADDRESS,
>> + sid, addr);
>> } else {
>> entry.perm = flag;
>> }
>> @@ -524,6 +573,9 @@ static IOMMUTLBEntry
>> virtio_iommu_translate(IOMMUMemoryRegion *mr
Re: [PATCH v12 05/13] virtio-iommu: Endpoint and domains structs and helpers
Hi Peter,
On 1/14/20 7:07 PM, Peter Xu wrote:
> On Tue, Jan 14, 2020 at 09:51:59AM +0100, Auger Eric wrote:
>> Hi Peter,
>
> Hi, Eric,
>
> [...]
>
>>>
+{
+VirtIOIOMMUEndpoint *ep;
+
+ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+if (ep) {
+return ep;
+}
+if (!virtio_iommu_mr(s, ep_id)) {
>>>
>>> Could I ask when this will trigger?
>>
>> This can happen when a device is attached to a domain and its RID does
>> not correspond to one of the devices protected by the iommu.
>
> So will it happen only because of a kernel driver bug?
>
> Also, I think the name of "virtio_iommu_mr" is confusing on that it
> returned explicitly a MemoryRegion however it's never been used:
>
> (since they're not in the same patch I'm pasting)
>
> static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
> {
> uint8_t bus_n, devfn;
> IOMMUPciBus *iommu_pci_bus;
> IOMMUDevice *dev;
>
> bus_n = PCI_BUS_NUM(sid);
> iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
> if (iommu_pci_bus) {
> devfn = sid & 0xFF;
> dev = iommu_pci_bus->pbdev[devfn];
> if (dev) {
> return &dev->iommu_mr;
> }
> }
> return NULL;
> }
>
> Maybe "return !!dev" would be enough, then make the return a boolean?
> Then we can rename it to virtio_iommu_has_device().
>
> PS. I think we can also drop IOMMU_PCI_DEVFN_MAX (after all you even
> didn't use it here!) and use PCI_DEVFN_MAX, and replace 0xFF.
Oh yes I can use PCI_DEVFN_MAX directly. Sorry.
Eric
>
> Thanks,
>
Re: [PATCH 0/4] Implements the NetBSD Virtual Machine Monitor accelerator
Ping? On 07.01.2020 13:53, Kamil Rytarowski wrote: > Hello QEMU Community! > > Over the past year the NetBSD team has been working hard on a new user-mode > API > for our hypervisor that will be released as part of the upcoming NetBSD 9.0. > This new API adds user-mode capabilities to create and manage virtual > machines, > configure memory mappings for guest machines, and create and control execution > of virtual processors. > > With this new API we are now able to bring our hypervisor to the QEMU > community! The following patches implement the NetBSD Virtual Machine Monitor > accelerator (NVMM) for QEMU on NetBSD 9.0 and newer hosts. > > When compiling QEMU for x86_64 passing the --enable-nvmm flag will compile the > accelerator for use. At runtime using the '-accel nvmm' should see a > significant performance improvement over emulation, much like when using 'hax' > on NetBSD. > > The documentation for this new API is visible at https://man.netbsd.org under > the libnvmm(3) and nvmm(4) pages. > > NVMM was designed and implemented by Maxime Villard. > > Thank you for your feedback. > > Maxime Villard (4): > Add the NVMM vcpu API > Add the NetBSD Virtual Machine Monitor accelerator. > Introduce the NVMM impl > Add the NVMM acceleration enlightenments > > accel/stubs/Makefile.objs |1 + > accel/stubs/nvmm-stub.c | 43 ++ > configure | 36 ++ > cpus.c| 58 ++ > include/sysemu/hw_accel.h | 14 + > include/sysemu/nvmm.h | 35 ++ > qemu-options.hx |4 +- > target/i386/Makefile.objs |1 + > target/i386/helper.c |2 +- > target/i386/nvmm-all.c| 1222 + > 10 files changed, 1413 insertions(+), 3 deletions(-) > create mode 100644 accel/stubs/nvmm-stub.c > create mode 100644 include/sysemu/nvmm.h > create mode 100644 target/i386/nvmm-all.c > > -- > 2.24.0 > signature.asc Description: OpenPGP digital signature
RE: [Xen-devel] [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
> -Original Message- > > > Linux PCI subsytem has an option resource_alignment that can be > > applied to either a single device or all devices. Booting with > > pci=resource_aligment=4096 will align each device to a page. Do you > > think pciback should force resource_alignment=4096 for dom0? > That sounds like a good idea. > Ideally Xen should keep track of the BARs position and size and refuse > to passthrough devices that have BARs sharing a page with other > devices BARs. > > > Are > > there other MMIO ranges to be concerned about adjacent to BARs? > > IIRC you can have two BARs of different devices in the same 4K page, > BARs are only aligned to it's size, so BARs smaller than 4K are not > required to be page aligned. If we had a notion of assignment groups for this, as well as devices sharing requester id, then Xen would not need to refuse pass-through, it would just require that all devices sharing the page were passed through as a unit. Paul
Re: Semihosting, arm, riscv, ppc and common code
On Wed, 15 Jan 2020 at 01:17, Benjamin Herrenschmidt wrote: > On Tue, 2020-01-14 at 09:59 +, Peter Maydell wrote: > > Note that semihosting is not a "here's a handy QEMU feature" > > thing. It's an architecture-specific API and ABI, which should > > be defined somewhere in a standard external to QEMU. > > There is no such standard for powerpc today that I know of. So you need to write one down somewhere. You're proposing an ABI which will be implemented by multiple implementors and used by multiple consumers. That needs a spec, not just code. I don't want to see more semihosting implementations added to QEMU that don't have a specification written down somewhere. The riscv single paragraph in their arch spec that vaguely says "works like arm semihosting" is not sufficient detail, incidentally. > Keith and I are somewhat of a different mind here. From the perspective > of the user of that API (picolibc is one), it's easier to deal with a > single one and have everybody inherit the same bugs. > > Now I understand the point of wanting to fix the mistakes made but I > would suggest we do so by proposing extensions to the existing one to > do so. The point about the mistakes is that you can't easily fix them by adding extensions, because the core of the biggest mistake is "we didn't provide a good way to allow extensions to be added and probed for by the user". So we had to implement an ugly and hard to implement mechanism instead. New architectures could mandate providing the good way from the start and avoid the painful-to-implement approach entirely. (I think RISCV has already missed this window of opportunity, which is a shame.) thanks -- PMM
Re: [PATCH v2 1/4] luks: extract block_crypto_calculate_payload_offset()
On Tue, Jan 14, 2020 at 04:25:44PM +0100, Max Reitz wrote:
> On 09.01.20 12:10, Stefan Hajnoczi wrote:
> > The qcow2 .bdrv_measure() code calculates the crypto payload offset.
> > This logic really belongs in block/crypto.c where it can be reused by
> > other image formats.
> >
> > The "luks" block driver will need this same logic in order to implement
> > .bdrv_measure(), so extract the block_crypto_calculate_payload_offset()
> > function now.
> >
> > Signed-off-by: Stefan Hajnoczi
> > ---
> > block/crypto.c | 64 ++
> > block/crypto.h | 5
> > block/qcow2.c | 59 --
> > 3 files changed, 73 insertions(+), 55 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 24823835c1..ed32202fa2 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -185,6 +185,70 @@ block_crypto_create_opts_init(QDict *opts, Error
> > **errp)
>
> [...]
>
> > +/* Determine the number of bytes for the crypto header */
> > +bool block_crypto_calculate_payload_offset(QemuOpts *opts,
> > + const char *optprefix,
> > + size_t *len,
> > + Error **errp)
> > +{
> > +QDict *cryptoopts_qdict;
> > +QCryptoBlockCreateOptions *cryptoopts;
> > +QCryptoBlock *crypto;
> > +
> > +/* Extract options into a qdict */
> > +if (optprefix) {
> > +QDict *opts_qdict = qemu_opts_to_qdict(opts, NULL);
> > +
> > +qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, optprefix);
> > +qobject_unref(opts_qdict);
> > +} else {
> > +cryptoopts_qdict = qemu_opts_to_qdict(opts, NULL);
> > +}
> > +
> > +/* Build QCryptoBlockCreateOptions object from qdict */
> > +qdict_put_str(cryptoopts_qdict, "format", "luks");
>
> Should this be a parameter?
Maybe one day, but there are no users who need it yet.
Stefan
signature.asc
Description: PGP signature
Re: [PATCH v2 2/4] luks: implement .bdrv_measure()
On Tue, Jan 14, 2020 at 04:43:36PM +0100, Max Reitz wrote:
> On 09.01.20 12:10, Stefan Hajnoczi wrote:
> > Add qemu-img measure support in the "luks" block driver.
> >
> > Signed-off-by: Stefan Hajnoczi
> > ---
> > block/crypto.c | 82 ++
> > 1 file changed, 82 insertions(+)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index ed32202fa2..51f37bb1f6 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -548,6 +548,87 @@ static int64_t block_crypto_getlength(BlockDriverState
> > *bs)
>
> [...]
>
> > +if (ret & BDRV_BLOCK_ZERO) {
> > +/* Skip zero regions */
> > +} else if ((ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) ==
> > + (BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED)) {
> > +/* Count clusters we've seen */
> > +required += pnum;
> > +}
>
> Don’t LUKS-encrypted files allocate effectively everything because zero
> data has to be encrypted, too?
>
> (“Effectively”, because you could zero out regions that are zero when
> encrypted, but...)
>
> > +}
> > +}
> > +
> > +/* Take into account preallocation. Nothing special is needed for
> > + * PREALLOC_MODE_METADATA since metadata is always counted.
> > + */
> > +if (prealloc == PREALLOC_MODE_FULL || prealloc ==
> > PREALLOC_MODE_FALLOC) {
> > +required = virtual_size;
>
> Same here. I think required should always be set to virtual_size.
Good points. I may have inherited this from the qcow2 code, where the
L2 tables can still have unallocated/zero clusters.
I'll check if this logic makes sense outside of qcow2.
Stefan
signature.asc
Description: PGP signature
Re: [PATCH] tests: acpi: update path in rebuild-expected-aml
On Wed, Jan 15, 2020 at 11:01:44AM +, Shameerali Kolothum Thodi wrote:
>
>
> > -Original Message-
> > From: Thomas Huth [mailto:th...@redhat.com]
> > Sent: 14 January 2020 17:08
> > To: Shameerali Kolothum Thodi ;
> > qemu-devel@nongnu.org; imamm...@redhat.com; m...@redhat.com
> > Cc: xuwei (O) ; Linuxarm
> > Subject: Re: [PATCH] tests: acpi: update path in rebuild-expected-aml
> >
> > On 14/01/2020 17.51, Shameer Kolothum wrote:
> > > Since commit 1e8a1fae7464("test: Move qtests to a separate
> > > directory") qtests are now placed in a separate folder and
> > > this breaks the script used to rebuild the expected ACPI
> > > tables for bios-tables-test. Update the script with correct
> > > path.
> > >
> > > Fixes: 1e8a1fae7464("test: Move qtests to a separate directory")
> > > Signed-off-by: Shameer Kolothum
> > > ---
> > > tests/data/acpi/rebuild-expected-aml.sh | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/data/acpi/rebuild-expected-aml.sh
> > b/tests/data/acpi/rebuild-expected-aml.sh
> > > index f89d4624bc..d44e511533 100755
> > > --- a/tests/data/acpi/rebuild-expected-aml.sh
> > > +++ b/tests/data/acpi/rebuild-expected-aml.sh
> > > @@ -14,7 +14,7 @@
> > >
> > > qemu_bins="x86_64-softmmu/qemu-system-x86_64
> > aarch64-softmmu/qemu-system-aarch64"
> > >
> > > -if [ ! -e "tests/bios-tables-test" ]; then
> > > +if [ ! -e "tests/qtest/bios-tables-test" ]; then
> > > echo "Test: bios-tables-test is required! Run make check before this
> > script."
> > > echo "Run this script from the build directory."
> > > exit 1;
> > > @@ -26,11 +26,11 @@ for qemu in $qemu_bins; do
> > > echo "Also, run this script from the build directory."
> > > exit 1;
> > > fi
> > > -TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu
> > tests/bios-tables-test
> > > +TEST_ACPI_REBUILD_AML=y QTEST_QEMU_BINARY=$qemu
> > tests/qtest/bios-tables-test
> > > done
> > >
> > > eval `grep SRC_PATH= config-host.mak`
> > >
> > > -echo '/* List of comma-separated changed AML files to ignore */' >
> > ${SRC_PATH}/tests/bios-tables-test-allowed-diff.h
> > > +echo '/* List of comma-separated changed AML files to ignore */' >
> > ${SRC_PATH}/tests/qtest/bios-tables-test-allowed-diff.h
> > >
> > > echo "The files were rebuilt and can be added to git."
> >
> > Oh, sorry for missing that in my patch series ... is there maybe a way
> > that we could test this script in one of our CI pipelines so that it is
> > not so easy to miss?
>
> Right. That will be a useful one.
>
> I am also seeing another error when I run "make check-qtest" on x86_64.
> This doesn’t seems to be related to the recent changes. I have gone back
> to 4.1.0 and it is still there.
>
> TESTcheck-qtest-x86_64: tests/boot-order-test
> TESTcheck-qtest-x86_64: tests/bios-tables-test
> Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> acpi-test: Warning! FACP binary file mismatch. Actual [aml:/tmp/aml-2Q9EE0],
> Expected [aml:tests/data/acpi/pc/FACP.bridge].
> acpi-test: Warning! FACP mismatch. Actual [asl:/tmp/asl-CQ9EE0.dsl,
> aml:/tmp/aml-2Q9EE0], Expected [asl:/tmp/asl-N18EE0.dsl,
> aml:tests/data/acpi/pc/FACP.bridge].
> **
> ERROR:tests/bios-tables-test.c:447:test_acpi_asl: assertion failed:
> (all_tables_match)
> ERROR - Bail out! ERROR:tests/bios-tables-test.c:447:test_acpi_asl: assertion
> failed: (all_tables_match)
> Aborted (core dumped)
> /home/shameer/qemu/tests/Makefile.include:899: recipe for target
> 'check-qtest-x86_64' failed
> make: *** [check-qtest-x86_64] Error 1
Well make check seems to pass for me ... What's different for you?
> FACP seems to have changed and it looks like need to run the script to
> generate
> a new one.
>
> ~/qemu$ diff -u /tmp/asl-CQ9EE0.dsl /tmp/asl-N18EE0.dsl
> --- /tmp/asl-CQ9EE0.dsl 2020-01-15 10:52:03.018448627 +
> +++ /tmp/asl-N18EE0.dsl 2020-01-15 10:52:03.022448627 +
> @@ -3,7 +3,7 @@
> * AML/ASL+ Disassembler version 20180105 (64-bit version)
> * Copyright (c) 2000 - 2018 Intel Corporation
> *
> - * Disassembly of /tmp/aml-2Q9EE0, Wed Jan 15 10:52:03 2020
> + * Disassembly of tests/data/acpi/pc/FACP.bridge, Wed Jan 15 10:52:03
> + 2020
> *
> * ACPI Data Table [FACP]
> *
> @@ -13,7 +13,7 @@
> [000h 4]Signature : "FACP"[Fixed ACPI
> Description Table (FADT)]
> [004h 0004 4] Table Length : 0074
> [008h 0008 1] Revisi
Re: [PATCH] qcow2: Use a GString in report_unsupported_feature()
On Tue 14 Jan 2020 07:08:04 PM CET, Alex Bennée wrote:
>g_autoptr(GString) features = g_string_sized_new(60);
>
> will save you the clean-up later.
Ok, will send v2 now.
>> +if (features->len > 0) {
>> +g_string_append(features, ", ");
>> +}
>> +g_string_append_printf(features, "%.46s",
>> table->name);
>
> We have a number of cases of this sort of idiom in the code base. I
> wonder if it calls for a utility function:
>
>qemu_append_with_sep(features, ", ", "%.46s", table->name)
Maybe it's worth checking. I can have a look once this is applied (there
was another similar one in the queue already).
Berto
Re: [PATCH v1] hw/i386: disable smbus migration for xenpv
On Mon, Jan 13, 2020 at 06:45:21PM +0100, Olaf Hering wrote: > With commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea a new member > smbus_no_migration_support was added, and enabled in two places. > With commit 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi > got new elements, which are conditionally filled. As a result, an > incoming migration expected smbus related data unless smbus migration > was disabled for a given MachineClass. > > Since commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle > xenpv, live migration to receiving hosts using qemu-4.0 and later is broken. > Therefore this patch must be applied to stable-4.x as well. > > Signed-off-by: Olaf Hering Thanks! I think you need to copy Xen maintainers to ack this. Suggest reposting with a fixed commit message and cc appropriate maintainers. > --- > hw/i386/pc_piix.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index fa12203079..e19716d0d3 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -952,6 +952,7 @@ static void xenfv_machine_options(MachineClass *m) > m->desc = "Xen Fully-virtualized PC"; > m->max_cpus = HVM_MAX_VCPUS; > m->default_machine_opts = "accel=xen"; > +m->smbus_no_migration_support = true; > } > > DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,
[PATCH v2] qcow2: Use a GString in report_unsupported_feature()
This is a bit more efficient than having to allocate and free memory
for each item.
The default size (60) is enough for all the existing incompatible
features or the "Unknown incompatible feature" message.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Alberto Garcia
Reviewed-by: Alex Bennée
---
block/qcow2.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
v2: Use g_autoptr and update commit message
diff --git a/block/qcow2.c b/block/qcow2.c
index cef9d72b3a..e29fc07068 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -453,16 +453,15 @@ static void cleanup_unknown_header_ext(BlockDriverState
*bs)
static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
uint64_t mask)
{
-char *features = g_strdup("");
-char *old;
+g_autoptr(GString) features = g_string_sized_new(60);
while (table && table->name[0] != '\0') {
if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
if (mask & (1ULL << table->bit)) {
-old = features;
-features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
- table->name);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features, "%.46s", table->name);
mask &= ~(1ULL << table->bit);
}
}
@@ -470,14 +469,14 @@ static void report_unsupported_feature(Error **errp,
Qcow2Feature *table,
}
if (mask) {
-old = features;
-features = g_strdup_printf("%s%sUnknown incompatible feature: %"
PRIx64,
- old, *old ? ", " : "", mask);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features,
+ "Unknown incompatible feature: %" PRIx64, mask);
}
-error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
-g_free(features);
+error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
}
/*
--
2.20.1
Re: Making QEMU easier for management tools and applications
Daniel P. Berrangé writes: > On Wed, Jan 15, 2020 at 01:15:17PM +0100, Markus Armbruster wrote: >> Christophe de Dinechin writes: >> >> > Thanks a bunch. This clarifies a number of my misconceptions about >> > how this is currently used. Most notably this one: >> > >> >> On 15 Jan 2020, at 10:20, Markus Armbruster wrote: >> >> >> >>> We don’t want the QAPI to let arbitrary fields of a QOM object >> >>> be modified, do we? >> >> >> >> We already do: QMP command qom-set. If it breaks your guest, you get to >> >> keep the pieces. >> > >> > Ouch. I certainly did not expect that. >> > >> > "It is not what you don’t know that kills you, it is what you know that >> > ain’t so". >> >> Do we have a legitimate use for qom-set right now? Hmm, let's check >> libvirt... aha: >> >> * qemuMonitorJSONSetMemoryStatsPeriod() uses it to control >> virtio-balloon's guest-stats-polling-interval property, in accordance >> with docs/virtio-balloon-stats.txt. >> >> * qemuMonitorJSONSetIOThread() uses it to control iothread's properties >> poll-max-ns, poll-grow, poll-shrink. Their use with -object is >> documented (in qemu-options.hx), their use with qom-set is not. >> >> Oh well. > > Libvirt is of course happy to switch to something else instead of > qom-set for these features if QEMU wants to provide a safer > alternative. Noted. libvirt's use of qom-set is okay. What's not okay is the near-complete lack of QOM documentation, its poor QMP interface, in part due to non-integration with QAPI, and last but not least the lack of QOM leadership leaving it adrift.
Re: [PATCH v2] qcow2: Use a GString in report_unsupported_feature()
On 1/15/20 2:56 PM, Alberto Garcia wrote:
This is a bit more efficient than having to allocate and free memory
for each item.
The default size (60) is enough for all the existing incompatible
features or the "Unknown incompatible feature" message.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Alberto Garcia
Reviewed-by: Alex Bennée
---
block/qcow2.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
v2: Use g_autoptr and update commit message
diff --git a/block/qcow2.c b/block/qcow2.c
index cef9d72b3a..e29fc07068 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -453,16 +453,15 @@ static void cleanup_unknown_header_ext(BlockDriverState
*bs)
static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
uint64_t mask)
{
-char *features = g_strdup("");
-char *old;
+g_autoptr(GString) features = g_string_sized_new(60);
while (table && table->name[0] != '\0') {
if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
if (mask & (1ULL << table->bit)) {
-old = features;
-features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
- table->name);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features, "%.46s", table->name);
mask &= ~(1ULL << table->bit);
}
}
@@ -470,14 +469,14 @@ static void report_unsupported_feature(Error **errp,
Qcow2Feature *table,
}
if (mask) {
-old = features;
-features = g_strdup_printf("%s%sUnknown incompatible feature: %"
PRIx64,
- old, *old ? ", " : "", mask);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features,
+ "Unknown incompatible feature: %" PRIx64, mask);
}
-error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
-g_free(features);
+error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
}
/*
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] qcow2: Use a GString in report_unsupported_feature()
On 1/14/20 10:35 PM, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
On 1/14/20 7:08 PM, Alex Bennée wrote:
Alberto Garcia writes:
This is a bit more efficient than having to allocate and free memory
for each item.
The default size (60) is enough for all the existing incompatible
features.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Alberto Garcia
---
block/qcow2.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index cef9d72b3a..ecf6827420 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -453,16 +453,15 @@ static void cleanup_unknown_header_ext(BlockDriverState
*bs)
static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
uint64_t mask)
{
-char *features = g_strdup("");
-char *old;
+GString *features = g_string_sized_new(60);
g_autoptr(GString) features = g_string_sized_new(60);
will save you the clean-up later.
Does this work with g_string_free() too?
It does:
static inline void g_autoptr_cleanup_gstring_free (GString *string)
{
if (string)
g_string_free (string, TRUE);
}
The implicit use of free_segment=TRUE was not obvious to me.
Thanks for checking it in glib source.
If you want to keep the allocated string but drop the GString you are
still best doing that yourself.
I agree. I asked because I had the other patch from Alberto in mind:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg669862.html
In this case we can not use the g_autoptr feature.
while (table && table->name[0] != '\0') {
if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
if (mask & (1ULL << table->bit)) {
-old = features;
-features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
- table->name);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features, "%.46s",
table->name);
We have a number of cases of this sort of idiom in the code base. I
wonder if it calls for a utility function:
qemu_append_with_sep(features, ", ", "%.46s", table->name)
Good idea for https://wiki.qemu.org/Contribute/BiteSizedTasks
Anyway not mandatory for this patch so with the autoptr fix:
Reviewed-by: Alex Bennée
mask &= ~(1ULL << table->bit);
}
}
@@ -470,14 +469,15 @@ static void report_unsupported_feature(Error **errp,
Qcow2Feature *table,
}
if (mask) {
-old = features;
-features = g_strdup_printf("%s%sUnknown incompatible feature: %"
PRIx64,
- old, *old ? ", " : "", mask);
-g_free(old);
+if (features->len > 0) {
+g_string_append(features, ", ");
+}
+g_string_append_printf(features,
+ "Unknown incompatible feature: %" PRIx64, mask);
}
-error_setg(errp, "Unsupported qcow2 feature(s): %s",
features);
-g_free(features);
+error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
+g_string_free(features, TRUE);
}
/*
Re: [PATCH v3 1/3] target/riscv: Fix tb->flags FS status
On Wed, Jan 15, 2020 at 2:29 PM Alistair Francis
wrote:
> > -*flags = cpu_mmu_index(env, 0);
> > -if (riscv_cpu_fp_enabled(env)) {
> > -*flags |= TB_FLAGS_MSTATUS_FS;
> > -}
> > +*flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
>
> I don't think this is right, you should use the riscv_cpu_fp_enabled()
> function.
>
> Right now it's the same as env->mstatus & MSTATUS_FS but when the
> Hypervisor extension goes in riscv_cpu_fp_enabled() will be more
> complex.
>
> Alistair
>
> I agree using riscv_cpu_fp_enabled() to hide the complexity when checking
FP,
but here I only duplicate the FP status (disabled/initial/clean/dirty) to
tb->flags
no matter FP is enabled or not.
Is it still necessary to check this before duplicating it?
I think it is not as long as TB_FLAGS_MSTATUS_FS is equivalent to
MSTATUS_FS.
But I don't know what changes hypervisor extension brings, please correct
me if I am wrong.
ShihPo
Re: [PATCH 051/104] virtiofsd: Parse flag FUSE_WRITE_KILL_PRIV
* Misono Tomohiro (misono.tomoh...@jp.fujitsu.com) wrote:
> > From: Vivek Goyal
> >
> > Caller can set FUSE_WRITE_KILL_PRIV in write_flags. Parse it and pass it
> > to the filesystem.
> >
> > Signed-off-by: Vivek Goyal
> > ---
> > tools/virtiofsd/fuse_common.h | 6 +-
> > tools/virtiofsd/fuse_lowlevel.c | 4 +++-
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
> > index 147c043bd9..1e8191b7a6 100644
> > --- a/tools/virtiofsd/fuse_common.h
> > +++ b/tools/virtiofsd/fuse_common.h
> > @@ -93,8 +93,12 @@ struct fuse_file_info {
> > */
> > unsigned int cache_readdir:1;
> >
> > +/* Indicates that suid/sgid bits should be removed upon write */
> > +unsigned int kill_priv:1;
> > +
> > +
> > /** Padding. Reserved for future use*/
> > -unsigned int padding:25;
> > +unsigned int padding:24;
> > unsigned int padding2:32;
> >
> > /*
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c
> > b/tools/virtiofsd/fuse_lowlevel.c
> > index bd5ca2f157..c8a3b1597a 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -1144,6 +1144,7 @@ static void do_write(fuse_req_t req, fuse_ino_t
> > nodeid,
> > memset(&fi, 0, sizeof(fi));
> > fi.fh = arg->fh;
> > fi.writepage = (arg->write_flags & FUSE_WRITE_CACHE) != 0;
> > +fi.kill_priv = !!(arg->write_flags & FUSE_WRITE_KILL_PRIV);
> >
> > fi.lock_owner = arg->lock_owner;
> > fi.flags = arg->flags;
> > @@ -1179,7 +1180,8 @@ static void do_write_buf(fuse_req_t req, fuse_ino_t
> > nodeid,
> > fi.lock_owner = arg->lock_owner;
> > fi.flags = arg->flags;
> > fi.fh = arg->fh;
> > -fi.writepage = arg->write_flags & FUSE_WRITE_CACHE;
> > +fi.writepage = !!(arg->write_flags & FUSE_WRITE_CACHE);
> > +fi.kill_priv = !!(arg->write_flags & FUSE_WRITE_KILL_PRIV);
> >
> > if (ibufv->count == 1) {
> > assert(!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD));
> > --
> > 2.23.0
>
> Reviewed-by: Misono Tomohiro
Thank you.
> side-note: virtiofs uses write_buf() and therefore do_write() is never called.
> How about cleanup the function?
Yes I think you're right; I need to go through and check there's no
corner case which can get into the plain do_write.
Dave
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v2 1/4] luks: extract block_crypto_calculate_payload_offset()
On 15.01.20 14:40, Stefan Hajnoczi wrote:
> On Tue, Jan 14, 2020 at 04:25:44PM +0100, Max Reitz wrote:
>> On 09.01.20 12:10, Stefan Hajnoczi wrote:
>>> The qcow2 .bdrv_measure() code calculates the crypto payload offset.
>>> This logic really belongs in block/crypto.c where it can be reused by
>>> other image formats.
>>>
>>> The "luks" block driver will need this same logic in order to implement
>>> .bdrv_measure(), so extract the block_crypto_calculate_payload_offset()
>>> function now.
>>>
>>> Signed-off-by: Stefan Hajnoczi
>>> ---
>>> block/crypto.c | 64 ++
>>> block/crypto.h | 5
>>> block/qcow2.c | 59 --
>>> 3 files changed, 73 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/block/crypto.c b/block/crypto.c
>>> index 24823835c1..ed32202fa2 100644
>>> --- a/block/crypto.c
>>> +++ b/block/crypto.c
>>> @@ -185,6 +185,70 @@ block_crypto_create_opts_init(QDict *opts, Error
>>> **errp)
>>
>> [...]
>>
>>> +/* Determine the number of bytes for the crypto header */
>>> +bool block_crypto_calculate_payload_offset(QemuOpts *opts,
>>> + const char *optprefix,
>>> + size_t *len,
>>> + Error **errp)
>>> +{
>>> +QDict *cryptoopts_qdict;
>>> +QCryptoBlockCreateOptions *cryptoopts;
>>> +QCryptoBlock *crypto;
>>> +
>>> +/* Extract options into a qdict */
>>> +if (optprefix) {
>>> +QDict *opts_qdict = qemu_opts_to_qdict(opts, NULL);
>>> +
>>> +qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, optprefix);
>>> +qobject_unref(opts_qdict);
>>> +} else {
>>> +cryptoopts_qdict = qemu_opts_to_qdict(opts, NULL);
>>> +}
>>> +
>>> +/* Build QCryptoBlockCreateOptions object from qdict */
>>> +qdict_put_str(cryptoopts_qdict, "format", "luks");
>>
>> Should this be a parameter?
>
> Maybe one day, but there are no users who need it yet.
Sure, but would it hurt? O:-)
I’m just asking because this file doesn’t implement luks crypto, so it
seems a bit strange to reference it here. Actually, now that I think
about it... This file only implements the luks block driver. Is this
even the right place for the common
block_crypto_calculate_payload_offset() function? Would it make more
sense in crypto/block.c or crypto/block-luks.c?
Max
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] hw/input: Do not enable CONFIG_PCKBD by default
On 1/15/20 12:46 PM, Laurent Vivier wrote: Le 15/01/2020 à 12:37, Thomas Huth a écrit : The i8042 device is part of the chipset of a machine, so it should be selected by the machines or chipsets (e.g. SuperIO chipsets), and not be enabled by default. The sentence "The i8042 device is part of the chipset of a machine" sounds odd to me. Maybe simply use "The i8042 PS/2 Controller should not be enabled by default. It has to be selected by machines or chipsets (e.g. SuperIO chipsets)." Signed-off-by: Thomas Huth --- hw/input/Kconfig | 1 - hw/isa/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/input/Kconfig b/hw/input/Kconfig index 287f08887b..a2f25725be 100644 --- a/hw/input/Kconfig +++ b/hw/input/Kconfig @@ -7,7 +7,6 @@ config LM832X config PCKBD bool -default y select PS2 depends on ISA_BUS diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index 8a38813cc1..c7f07854f7 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -16,6 +16,7 @@ config I82378 config ISA_SUPERIO bool select ISA_BUS +select PCKBD config PC87312 bool It is also used by R4K but the Kconfig already includes it, so: I believe R4K should 'select ISA_SUPERIO' but we don't have specs and this machine is deprecated, so I didn't bother improving it. Reviewed-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 1/4] qapi: Add a 'coroutine' flag for commands
Kevin Wolf writes:
> This patch adds a new 'coroutine' flag to QMP command definitions that
> tells the QMP dispatcher that the command handler is safe to be run in a
> coroutine.
>
> Signed-off-by: Kevin Wolf
> Reviewed-by: Marc-André Lureau
> ---
> tests/qapi-schema/qapi-schema-test.json | 1 +
> docs/devel/qapi-code-gen.txt| 4
> include/qapi/qmp/dispatch.h | 1 +
> tests/test-qmp-cmds.c | 4
> scripts/qapi/commands.py| 17 +++--
> scripts/qapi/doc.py | 2 +-
> scripts/qapi/expr.py| 4 ++--
> scripts/qapi/introspect.py | 2 +-
> scripts/qapi/schema.py | 9 ++---
> tests/qapi-schema/qapi-schema-test.out | 2 ++
> tests/qapi-schema/test-qapi.py | 7 ---
> 11 files changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/tests/qapi-schema/qapi-schema-test.json
> b/tests/qapi-schema/qapi-schema-test.json
> index 9abf175fe0..1a850fe171 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -147,6 +147,7 @@
>'returns': 'UserDefTwo' }
>
> { 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
> +{ 'command': 'coroutine-cmd', 'data': {}, 'coroutine': true }
>
> # Returning a non-dictionary requires a name from the whitelist
> { 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 45c93a43cc..753f6711d3 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -457,6 +457,7 @@ Syntax:
> '*gen': false,
> '*allow-oob': true,
> '*allow-preconfig': true,
> +'*coroutine': true,
> '*if': COND,
> '*features': FEATURES }
>
> @@ -581,6 +582,9 @@ before the machine is built. It defaults to false. For
> example:
> QMP is available before the machine is built only when QEMU was
> started with --preconfig.
>
> +Member 'coroutine' tells the QMP dispatcher whether the command handler
> +is safe to be run in a coroutine. It defaults to false.
Two spaces after sentence-ending period, for consistency with the rest
of the file.
As discussed in review of prior versions, coroutine-safety is an
implementation detail that should not be exposed to management
applications. Therefore, we want a flag, not a feature.
As far as I can tell, the new flag has no effect until PATCH 3 puts it
to use. That's okay.
The doc update tells us when we may say 'coroutine': true, namely when
the handler function is coroutine-safe. It doesn't quite tell us what
difference it makes, or rather will make after PATCH 3. I think it
should.
In review of a prior version, Marc-André wondered whether keeping
allow-oob and coroutine separate makes sense. Recall qapi-code-gen.txt:
An OOB-capable command handler must satisfy the following conditions:
- It terminates quickly.
- It does not invoke system calls that may block.
- It does not access guest RAM that may block when userfaultfd is
enabled for postcopy live migration.
- It takes only "fast" locks, i.e. all critical sections protected by
any lock it takes also satisfy the conditions for OOB command
handler code.
The restrictions on locking limit access to shared state. Such access
requires synchronization, but OOB commands can't take the BQL or any
other "slow" lock.
Kevin, does this rule out coroutine use?
> +
> The optional 'if' member specifies a conditional. See "Configuring
> the schema" below for more on this.
>
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 9aa426a398..d6ce9efc8e 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -24,6 +24,7 @@ typedef enum QmpCommandOptions
> QCO_NO_SUCCESS_RESP = (1U << 0),
> QCO_ALLOW_OOB = (1U << 1),
> QCO_ALLOW_PRECONFIG = (1U << 2),
> +QCO_COROUTINE = (1U << 3),
> } QmpCommandOptions;
>
> typedef struct QmpCommand
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index 27b0afe55a..e2f71e42a1 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -34,6 +34,10 @@ void qmp_cmd_success_response(Error **errp)
> {
> }
>
> +void qmp_coroutine_cmd(Error **errp)
> +{
> +}
> +
> Empty2 *qmp_user_def_cmd0(Error **errp)
> {
> return g_new0(Empty2, 1);
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index ab98e504f3..97e51401f1 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -15,6 +15,7 @@ See the COPYING file in the top-level directory.
>
> from qapi.common import *
> from qapi.gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext
> +from typing import List
>
>
> def gen_comma
Re: [PATCH v12 05/13] virtio-iommu: Endpoint and domains structs and helpers
On Wed, Jan 15, 2020 at 02:00:22PM +0100, Auger Eric wrote:
> Hi Peter,
>
>
> On 1/14/20 7:07 PM, Peter Xu wrote:
> > On Tue, Jan 14, 2020 at 09:51:59AM +0100, Auger Eric wrote:
> >> Hi Peter,
> >
> > Hi, Eric,
> >
> > [...]
> >
> >>>
> +{
> +VirtIOIOMMUEndpoint *ep;
> +
> +ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
> +if (ep) {
> +return ep;
> +}
> +if (!virtio_iommu_mr(s, ep_id)) {
> >>>
> >>> Could I ask when this will trigger?
> >>
> >> This can happen when a device is attached to a domain and its RID does
> >> not correspond to one of the devices protected by the iommu.
>
> >
> > So will it happen only because of a kernel driver bug?
>
> Yes, at the moment, because virtio_iommu_mr() only gets called on device
> attach to a domain.
>
> The spec says:
> "If the endpoint identified by endpoint doesn’t exist, the device MUST
> reject the request and set status to VIRTIO_IOMMU_S_NOENT"
Sure. I just wanted to make sure I didn't miss anything, because I
really can't see when this extra logic can help, say, right now we
only have one vIOMMU at least for VT-d, so all devices will be managed
by that. But yeah if that's explicitly mentioned in the spec, I'd
agree we should follow that.
> >
> > Also, I think the name of "virtio_iommu_mr" is confusing on that it
> > returned explicitly a MemoryRegion however it's never been used:
>
> I use the same prototype as for smmu_iommu_mr(). Returning the iommu mr
> will allow to proceed with further RID based operations like invalidations.
>
> The same logic is used in vtd_context_device_invalidate.
I'm fine with this. Let's keep virtio_iommu_mr() as you prefer.
Another thing I'd like to mention is that, I don't think "the same
logic is used in VT-d" matters much. If we think something is wrong
(even if it's the same in VT-d), why not we fix both? :-)
Thanks,
>
>
> >
> > (since they're not in the same patch I'm pasting)
> >
> > static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
> > {
> > uint8_t bus_n, devfn;
> > IOMMUPciBus *iommu_pci_bus;
> > IOMMUDevice *dev;
> >
> > bus_n = PCI_BUS_NUM(sid);
> > iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
> > if (iommu_pci_bus) {
> > devfn = sid & 0xFF;
> > dev = iommu_pci_bus->pbdev[devfn];
> > if (dev) {
> > return &dev->iommu_mr;
> > }
> > }
> > return NULL;
> > }
> >
> > Maybe "return !!dev" would be enough, then make the return a boolean?
> > Then we can rename it to virtio_iommu_has_device().
> >
> > PS. I think we can also drop IOMMU_PCI_DEVFN_MAX (after all you even
> > didn't use it here!) and use PCI_DEVFN_MAX, and replace 0xFF.
> well intel iommu and smmu use a similar constant (PCI_DEVFN_MAX,
> SMMU_PCI_DEVFN_MAX resp.). I use it in virtio_iommu_find_add_as
--
Peter Xu
Re: [PATCH v12 09/13] virtio-iommu: Implement fault reporting
On Wed, Jan 15, 2020 at 02:12:20PM +0100, Auger Eric wrote:
> >> +static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
> >> + int flags, uint32_t endpoint,
> >> + uint64_t address)
> >> +{
[...]
> >> +if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
> >> +virtio_error(vdev, "error buffer of wrong size");
> >> +virtqueue_detach_element(vq, elem, 0);
> >> +g_free(elem);
> >> +continue;
> >
> > If virtio_error(), should we stop rather than continue?
> My understanding is the buffer just popped had a wrong size so it is not
> usable. We skip it we try to use another one if any. Does it make sense?
I'm not very familiar to virtio, but I see that virtio_error marks
vdev->broken to true. If with that iiuc the next virtqueue_pop() will
fail directly (see the first call to virtio_device_disabled in
virtqueue_pop). Then I don't see why retry any more...
Thanks,
--
Peter Xu
[PATCH v2 02/86] machine: introduce ram-memdev property
Property will contain link to memory backend that will be
used for backing initial RAM.
Follow up commit will alias -mem-path and -mem-prealloc
CLI options into memory backend options to make memory
handling consistent (using only hostmem backend family
for guest RAM allocation).
Signed-off-by: Igor Mammedov
---
CC: ehabk...@redhat.com
CC: pbonz...@redhat.com
---
include/hw/boards.h | 2 ++
hw/core/machine.c | 9 +
2 files changed, 11 insertions(+)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb1b43d..6aa01b8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -4,6 +4,7 @@
#define HW_BOARDS_H
#include "exec/memory.h"
+#include "sysemu/hostmem.h"
#include "sysemu/blockdev.h"
#include "sysemu/accel.h"
#include "qapi/qapi-types-machine.h"
@@ -285,6 +286,7 @@ struct MachineState {
bool enforce_config_section;
bool enable_graphics;
char *memory_encryption;
+HostMemoryBackend *ram_memdev;
DeviceMemoryState *device_memory;
ram_addr_t ram_size;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 3e288bf..fd573d2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -886,6 +886,15 @@ static void machine_initfn(Object *obj)
"Table (HMAT)", NULL);
}
+object_property_add_link(obj, "ram-memdev", TYPE_MEMORY_BACKEND,
+ (Object **)&ms->ram_memdev,
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_STRONG, &error_abort);
+object_property_set_description(obj, "ram-memdev",
+"Set RAM backend"
+"Valid value is ID of hostmem based
backend",
+ &error_abort);
+
/* Register notifier when init is done for sysbus sanity checks */
ms->sysbus_notifier.notify = machine_init_notify;
qemu_add_machine_init_done_notifier(&ms->sysbus_notifier);
--
2.7.4
[PATCH v2 01/86] numa: remove deprecated -mem-path fallback to anonymous RAM
it was deprecated since 4.0 by commit
cb79224b7 (deprecate -mem-path fallback to anonymous RAM)
Deprecation period ran ont and it's time to remove it
so it won't get in a way of switching to using hostmem
backend for RAM.
Signed-off-by: Igor Mammedov
---
CC:libvir-l...@redhat.com
CC: ehabk...@redhat.com
---
hw/core/numa.c | 18 +-
qemu-deprecated.texi | 9 -
2 files changed, 1 insertion(+), 26 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 0d1b4be..840e685 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -784,24 +784,8 @@ static void allocate_system_memory_nonnuma(MemoryRegion
*mr, Object *owner,
{
if (mem_path) {
#ifdef __linux__
-Error *err = NULL;
memory_region_init_ram_from_file(mr, owner, name, ram_size, 0, 0,
- mem_path, &err);
-if (err) {
-error_report_err(err);
-if (mem_prealloc) {
-exit(1);
-}
-warn_report("falling back to regular RAM allocation");
-error_printf("This is deprecated. Make sure that -mem-path "
- " specified path has sufficient resources to allocate"
- " -m specified RAM amount\n");
-/* Legacy behavior: if allocation failed, fall back to
- * regular RAM allocation.
- */
-mem_path = NULL;
-memory_region_init_ram_nomigrate(mr, owner, name, ram_size,
&error_fatal);
-}
+ mem_path, &error_fatal);
#else
fprintf(stderr, "-mem-path not supported on this host\n");
exit(1);
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 0968d37..982af95 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -113,15 +113,6 @@ QEMU using implicit generic or board specific splitting
rule.
Use @option{memdev} with @var{memory-backend-ram} backend or @option{mem} (if
it's supported by used machine type) to define mapping explictly instead.
-@subsection -mem-path fallback to RAM (since 4.1)
-Currently if guest RAM allocation from file pointed by @option{mem-path}
-fails, QEMU falls back to allocating from RAM, which might result
-in unpredictable behavior since the backing file specified by the user
-is ignored. In the future, users will be responsible for making sure
-the backing storage specified with @option{-mem-path} can actually provide
-the guest RAM configured with @option{-m} and QEMU will fail to start up if
-RAM allocation is unsuccessful.
-
@subsection RISC-V -bios (since 4.1)
QEMU 4.1 introduced support for the -bios option in QEMU for RISC-V for the
--
2.7.4
[PATCH v2 04/86] machine: introduce convenience MachineState::ram
the new field will be used by boards to get access to main
RAM memory region and will help to save boiler plate in
boards which often add a field or variable just for this
purpose.
Memory region will be equivalent to what currently used
memory_region_allocate_system_memory() is returning apart
from that it will come from hostmem backend.
Followup patches will incrementally switch boards to using
RAM from MachineState::ram.
Patch takes care of non-NUMA case and follow up patch will
initialize MachineState::ram for NUMA case.
Signed-off-by: Igor Mammedov
---
CC: ehabk...@redhat.com
CC: pbonz...@redhat.com
---
include/hw/boards.h | 9 -
hw/core/machine.c | 21 +
hw/core/numa.c | 14 +-
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index e3ea8b0..80d73b2 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -73,7 +73,12 @@ void machine_set_cpu_numa_node(MachineState *machine,
Error **errp);
void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char
*type);
-
+/*
+ * Checks that backend isn't used, preps it for exclusive usage and
+ * returns migratable MemoryRegion provided by backend.
+ */
+MemoryRegion *machine_consume_memdev(MachineState *machine,
+ HostMemoryBackend *backend);
/**
* CPUArchId:
@@ -292,6 +297,8 @@ struct MachineState {
bool enable_graphics;
char *memory_encryption;
HostMemoryBackend *ram_memdev;
+/* convenience alias to ram_memdev memory region or numa container */
+MemoryRegion *ram;
DeviceMemoryState *device_memory;
ram_addr_t ram_size;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index fd573d2..9a43081 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -26,6 +26,7 @@
#include "sysemu/qtest.h"
#include "hw/pci/pci.h"
#include "hw/mem/nvdimm.h"
+#include "migration/vmstate.h"
GlobalProperty hw_compat_4_2[] = {
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
@@ -1041,10 +1042,30 @@ static void machine_numa_finish_cpu_init(MachineState
*machine)
g_string_free(s, true);
}
+MemoryRegion *machine_consume_memdev(MachineState *machine,
+ HostMemoryBackend *backend)
+{
+MemoryRegion *ret = host_memory_backend_get_memory(backend);
+
+if (memory_region_is_mapped(ret)) {
+char *path = object_get_canonical_path_component(OBJECT(backend));
+error_report("memory backend %s can't be used multiple times.", path);
+g_free(path);
+exit(EXIT_FAILURE);
+}
+host_memory_backend_set_mapped(backend, true);
+vmstate_register_ram_global(ret);
+return ret;
+}
+
void machine_run_board_init(MachineState *machine)
{
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
+if (machine->ram_memdev) {
+machine->ram = machine_consume_memdev(machine, machine->ram_memdev);
+}
+
if (machine->numa_state) {
numa_complete_configuration(machine);
if (machine->numa_state->num_nodes) {
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 840e685..8264336 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -817,20 +817,8 @@ void memory_region_allocate_system_memory(MemoryRegion
*mr, Object *owner,
if (!backend) {
continue;
}
-MemoryRegion *seg = host_memory_backend_get_memory(backend);
-
-if (memory_region_is_mapped(seg)) {
-char *path = object_get_canonical_path_component(OBJECT(backend));
-error_report("memory backend %s is used multiple times. Each "
- "-numa option must use a different memdev value.",
- path);
-g_free(path);
-exit(1);
-}
-
-host_memory_backend_set_mapped(backend, true);
+MemoryRegion *seg = machine_consume_memdev(ms, backend);
memory_region_add_subregion(mr, addr, seg);
-vmstate_register_ram_global(seg);
addr += size;
}
}
--
2.7.4
[PATCH v2 03/86] machine: alias -mem-path and -mem-prealloc into memory-foo backend
Allow a machine to opt in for hostmem backend based initial
RAM even if user used old -mem-path/prealloc options by providing
MachineClass::default_ram_id
Follow up patches will incrementally convert machines to new API,
by dropping memory_region_allocate_system_memory() and setting
default_ram_id that board used to use before conversion to keep
migration stream the same.
Signed-off-by: Igor Mammedov
---
CC: ehabk...@redhat.com
CC: pbonz...@redhat.com
---
include/hw/boards.h | 5 +
include/sysemu/hostmem.h | 16
backends/hostmem-file.c | 7 ---
backends/hostmem-ram.c | 2 --
vl.c | 25 +
5 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6aa01b8..e3ea8b0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -170,6 +170,10 @@ typedef struct {
*false is returned, an error must be set to show the reason of
*the rejection. If the hook is not provided, all hotplug will be
*allowed.
+ * @default_ram_id:
+ *Specifies inital RAM MemoryRegion name to be used for default backend
+ *creation if user explicitly hasn't specified backend with "ram-memdev"
+ *property.
*/
struct MachineClass {
/*< private >*/
@@ -226,6 +230,7 @@ struct MachineClass {
bool nvdimm_supported;
bool numa_mem_supported;
bool auto_enable_numa;
+const char *default_ram_id;
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 4dbdadd..5db0d66 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -27,6 +27,22 @@
#define MEMORY_BACKEND_CLASS(klass) \
OBJECT_CLASS_CHECK(HostMemoryBackendClass, (klass), TYPE_MEMORY_BACKEND)
+/* hostmem-ram.c */
+/**
+ * @TYPE_MEMORY_BACKEND_RAM:
+ * name of backend that uses mmap on the anonymous RAM
+ */
+
+#define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
+
+/* hostmem-file.c */
+/**
+ * @TYPE_MEMORY_BACKEND_FILE:
+ * name of backend that uses mmap on a file descriptor
+ */
+#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
+
+typedef struct HostMemoryBackend HostMemoryBackend;
typedef struct HostMemoryBackendClass HostMemoryBackendClass;
/**
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index be64020..cb319a9 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -18,13 +18,6 @@
#include "sysemu/sysemu.h"
#include "qom/object_interfaces.h"
-/* hostmem-file.c */
-/**
- * @TYPE_MEMORY_BACKEND_FILE:
- * name of backend that uses mmap on a file descriptor
- */
-#define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
-
#define MEMORY_BACKEND_FILE(obj) \
OBJECT_CHECK(HostMemoryBackendFile, (obj), TYPE_MEMORY_BACKEND_FILE)
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 6aab8d3..5cc53e7 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -16,8 +16,6 @@
#include "qemu/module.h"
#include "qom/object_interfaces.h"
-#define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
-
static void
ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
diff --git a/vl.c b/vl.c
index 7514012..d36a857 100644
--- a/vl.c
+++ b/vl.c
@@ -75,6 +75,7 @@ int main(int argc, char **argv)
#include "ui/input.h"
#include "sysemu/sysemu.h"
#include "sysemu/numa.h"
+#include "sysemu/hostmem.h"
#include "exec/gdbstub.h"
#include "qemu/timer.h"
#include "chardev/char.h"
@@ -2820,6 +2821,25 @@ static void configure_accelerators(const char *progname)
}
}
+static void create_default_memdev(MachineState *ms, const char *path,
+ bool prealloc)
+{
+Object *obj;
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+obj = object_new(path ? TYPE_MEMORY_BACKEND_FILE :
TYPE_MEMORY_BACKEND_RAM);
+if (path) {
+object_property_set_str(obj, path, "mem-path", &error_fatal);
+}
+object_property_set_bool(obj, prealloc, "prealloc", &error_fatal);
+object_property_set_int(obj, ms->ram_size, "size", &error_fatal);
+object_property_add_child(object_get_objects_root(), mc->default_ram_id,
+ obj, &error_fatal);
+user_creatable_complete(USER_CREATABLE(obj), &error_fatal);
+object_unref(obj);
+object_property_set_link(OBJECT(ms), obj, "ram-memdev", &error_fatal);
+}
+
int main(int argc, char **argv, char **envp)
{
int i;
@@ -4283,6 +4303,11 @@ int main(int argc, char **argv, char **envp)
}
parse_numa_opts(current_machine);
+if (!current_machine->ram_memdev &&
+ machine_class->default_ram_size &&
+ machine_class->default_ram_id) {
+create_default_memdev(current_machine, mem_path, mem_prealloc);
+}
/* do monitor/qmp handling at preconfig state if requested */
main_loop();
--
2.7.4
[PATCH v2 00/86] refactor main RAM allocation to use hostmem backend
v2: - fix compile errors on mingw32 host by introducing RAM_ADDR_UFMT [11/86] - replace "[PATCH 43/86] hppa: drop RAM size fixup" with alternative patches made by Philippe (which effectively do the same thing but other way around) - ppc440: fix crash and add suggested valid RAM size in error output. s/ppc4xx_sdram_adjust/ppc4xx_sdram_prep/ and simplify it by removing not necessary nested loop - rebase on current master due to new conflicts Series removes ad hoc RAM allocation API (memory_region_allocate_system_memory) and consolidates it around hostmem backend. It allows to * resolve conflicts between global -mem-prealloc and hostmem's "policy" option fixing premature allocation before binding policy is applied * simplify complicated memory allocation routines which had to deal with 2 ways to allocate RAM. * it allows to reuse hostmem backends of a choice for main RAM without adding extra CLI options to duplicate hostmem features. Recent case was -mem-shared, to enable vhost-user on targets that don't support hostmem backends [1] (ex: s390) * move RAM allocation from individual boards into generic machine code and provide them with prepared MemoryRegion. * clean up deprecated NUMA features which were tied to the old API (see patches) - "numa: remove deprecated -mem-path fallback to anonymous RAM" - "numa: remove deprecated implicit RAM distribution between nodes" - "forbid '-numa node,mem' for 5.0 and newer machine types" Conversion introduces a new machine.ram-memdev property and wrapper code that aliases global -mem-path and -mem-alloc into automatically created hostmem backend properties (provided ram-memdev was not set explicitly given by user). And then follows bulk of trivial patches that incrementally convert individual boards to using machine.ram-memdev provided MemoryRegion. Board conversion typically involves: * providing MachineClass::default_ram_size and MachineClass::default_ram_id so generic code could create default backend if user didn't explicitly provide ram-memdev or -m options * dropping memory_region_allocate_system_memory() call * using convenience MachineState::ram MemoryRegion, which points to MemoryRegion allocated by ram-memdev On top of that for some boards: * added missing ram_size checks (typically it were boards with fixed ram size) * ram_size fixups were replaced by checks and hard errors, forcing user to provide correct "-m" values instead of ignoring it and continuing running. After all boards are converted the old API is removed and memory allocation routines are cleaned up. git tree for testing: https://github.com/imammedo/qemu convert_main_ram_to_memdev_v2 previous rev: https://github.com/imammedo/qemu convert_main_ram_to_memdev_v2 https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg05409.html Igor Mammedov (83): numa: remove deprecated -mem-path fallback to anonymous RAM machine: introduce ram-memdev property machine: alias -mem-path and -mem-prealloc into memory-foo backend machine: introduce convenience MachineState::ram initialize MachineState::ram in NUMA case alpha:dp264: use memdev for RAM arm:aspeed: convert valid RAM sizes to data arm:aspeed: actually check RAM size hw:aspeed: drop warning and bogus ram_size fixup arm:aspeed: use memdev for RAM arm:collie: use memdev for RAM arm:cubieboard: use memdev for RAM arm:digic_boards: use memdev for RAM arm:highbank: use memdev for RAM arm:imx25_pdk: drop RAM size fixup arm:imx25_pdk: use memdev for RAM arm:integratorcp: use memdev for RAM arm:kzm: drop RAM size fixup arm:kzm: use memdev for RAM arm:mcimx6ul-evk: use memdev for RAM arm:mcimx7d-sabre: use memdev for RAM arm:mps2-tz: use memdev for RAM arm:mps2: use memdev for RAM arm:musicpal: use memdev for RAM arm:nseries: use memdev for RAM arm:omap_sx1: use memdev for RAM arm:palm: use memdev for RAM arm:raspi: use memdev for RAM arm:sabrelite: use memdev for RAM arm:sbsa-ref: use memdev for RAM arm:versatilepb: use memdev for RAM arm:vexpress: use memdev for RAM arm:virt: use memdev for RAM arm:xilinx_zynq: drop RAM size fixup arm:xilinx_zynq: use memdev for RAM arm:xlnx-versal-virt: use memdev for RAM arm:xlnx-zcu102: use memdev for RAM s390x:s390-virtio-ccw: use memdev for RAM null-machine: use memdev for RAM cris:axis_dev88: use memdev for RAM hppa: use memdev for RAM x86:microvm: use memdev for RAM x86:pc: use memdev for RAM lm32:lm32_boards: use memdev for RAM lm32:milkymist: use memdev for RAM m68k:an5206: use memdev for RAM m68k:mcf5208: use memdev for RAM m68k:next-cube: use memdev for RAM mips:boston-cube: use memdev for RAM mips:mips_fulong2e: drop RAM size fixup mips:mips_fulong2e: use memdev for RAM mips:mips_jazz: use memdev for RAM mips:mips_malta: use memdev for RAM mips:mips_mipssim: use memdev for RAM mips:mips_r4k: use memdev for RAM ppc
[PATCH v2 07/86] arm:aspeed: convert valid RAM sizes to data
various foo_rambits() hardcode mapping of RAM sizes to RAM feature bits,
which is hard to reuse and repeats over and over.
Convert maps into GLib's hash tables and perform mapping using
common mapping function.
Follow up patch will reuse tables for actually checking ram-size
property.
Signed-off-by: Igor Mammedov
---
CC: c...@kaod.org
CC: peter.mayd...@linaro.org
CC: and...@aj.id.au
CC: j...@jms.id.au
CC: qemu-...@nongnu.org
---
include/hw/misc/aspeed_sdmc.h | 2 +
hw/misc/aspeed_sdmc.c | 116 --
2 files changed, 47 insertions(+), 71 deletions(-)
diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
index 5dbde59..de1501f 100644
--- a/include/hw/misc/aspeed_sdmc.h
+++ b/include/hw/misc/aspeed_sdmc.h
@@ -39,6 +39,8 @@ typedef struct AspeedSDMCState {
typedef struct AspeedSDMCClass {
SysBusDeviceClass parent_class;
+GHashTable *ram2feat;
+int fallback_ram_size;
uint64_t max_ram_size;
uint32_t (*compute_conf)(AspeedSDMCState *s, uint32_t data);
void (*write)(AspeedSDMCState *s, uint32_t reg, uint32_t data);
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 2df3244..3fc80f0 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -148,72 +148,6 @@ static const MemoryRegionOps aspeed_sdmc_ops = {
.valid.max_access_size = 4,
};
-static int ast2400_rambits(AspeedSDMCState *s)
-{
-switch (s->ram_size >> 20) {
-case 64:
-return ASPEED_SDMC_DRAM_64MB;
-case 128:
-return ASPEED_SDMC_DRAM_128MB;
-case 256:
-return ASPEED_SDMC_DRAM_256MB;
-case 512:
-return ASPEED_SDMC_DRAM_512MB;
-default:
-break;
-}
-
-/* use a common default */
-warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 256M",
-s->ram_size);
-s->ram_size = 256 << 20;
-return ASPEED_SDMC_DRAM_256MB;
-}
-
-static int ast2500_rambits(AspeedSDMCState *s)
-{
-switch (s->ram_size >> 20) {
-case 128:
-return ASPEED_SDMC_AST2500_128MB;
-case 256:
-return ASPEED_SDMC_AST2500_256MB;
-case 512:
-return ASPEED_SDMC_AST2500_512MB;
-case 1024:
-return ASPEED_SDMC_AST2500_1024MB;
-default:
-break;
-}
-
-/* use a common default */
-warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M",
-s->ram_size);
-s->ram_size = 512 << 20;
-return ASPEED_SDMC_AST2500_512MB;
-}
-
-static int ast2600_rambits(AspeedSDMCState *s)
-{
-switch (s->ram_size >> 20) {
-case 256:
-return ASPEED_SDMC_AST2600_256MB;
-case 512:
-return ASPEED_SDMC_AST2600_512MB;
-case 1024:
-return ASPEED_SDMC_AST2600_1024MB;
-case 2048:
-return ASPEED_SDMC_AST2600_2048MB;
-default:
-break;
-}
-
-/* use a common default */
-warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 1024M",
-s->ram_size);
-s->ram_size = 1024 << 20;
-return ASPEED_SDMC_AST2600_1024MB;
-}
-
static void aspeed_sdmc_reset(DeviceState *dev)
{
AspeedSDMCState *s = ASPEED_SDMC(dev);
@@ -257,11 +191,14 @@ static Property aspeed_sdmc_properties[] = {
static void aspeed_sdmc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass);
+
dc->realize = aspeed_sdmc_realize;
dc->reset = aspeed_sdmc_reset;
dc->desc = "ASPEED SDRAM Memory Controller";
dc->vmsd = &vmstate_aspeed_sdmc;
dc->props = aspeed_sdmc_properties;
+asc->ram2feat = g_hash_table_new(g_direct_hash, g_direct_equal);
}
static const TypeInfo aspeed_sdmc_info = {
@@ -273,10 +210,28 @@ static const TypeInfo aspeed_sdmc_info = {
.abstract = true,
};
+static int aspeed_get_ram_feat(AspeedSDMCState *s)
+{
+AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
+int ram_mb = s->ram_size >> 20;
+gpointer val;
+
+if (g_hash_table_contains(asc->ram2feat, GINT_TO_POINTER(ram_mb))) {
+val = g_hash_table_lookup(asc->ram2feat, GINT_TO_POINTER(ram_mb));
+return GPOINTER_TO_INT(val);
+}
+
+warn_report("Invalid RAM size 0x%" PRIx64 ". Using default %dM",
+ s->ram_size, asc->fallback_ram_size);
+s->ram_size = asc->fallback_ram_size << 20;
+val = g_hash_table_lookup(asc->ram2feat, &asc->fallback_ram_size);
+return GPOINTER_TO_INT(val);
+}
+
static uint32_t aspeed_2400_sdmc_compute_conf(AspeedSDMCState *s, uint32_t
data)
{
-uint32_t fixed_conf = ASPEED_SDMC_VGA_COMPAT |
-ASPEED_SDMC_DRAM_SIZE(ast2400_rambits(s));
+int ram_f = aspeed_get_ram_feat(s);
+uint32_t fixed_conf = ASPEED_SDMC_VGA_COMPAT |
ASPEED_SDMC_DRAM_SIZE(ram_f);
/* Make sure readonly bits are kept */
data &= ~ASPEED_SDMC_READONLY_MASK;
@@ -298,6 +253,9 @@ static void aspeed_2400_sdmc_write(AspeedSDMCState *s,
uint32_t reg,
s->regs[reg] = data;
}
+#defin
[PATCH v2 06/86] alpha:dp264: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Acked-by: Richard Henderson
---
hw/alpha/alpha_sys.h | 2 +-
hw/alpha/dp264.c | 3 ++-
hw/alpha/typhoon.c | 8 ++--
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 95033d7..bc0a286 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,7 +11,7 @@
#include "hw/intc/i8259.h"
-PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
+PCIBus *typhoon_init(MemoryRegion *, ISABus **, qemu_irq *, AlphaCPU *[4],
pci_map_irq_fn);
/* alpha_pci.c. */
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index f2026fd..29439c7 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -75,7 +75,7 @@ static void clipper_init(MachineState *machine)
cpus[0]->env.trap_arg2 = smp_cpus;
/* Init the chipset. */
-pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus,
+pci_bus = typhoon_init(machine->ram, &isa_bus, &rtc_irq, cpus,
clipper_pci_map_irq);
/* Since we have an SRM-compatible PALcode, use the SRM epoch. */
@@ -183,6 +183,7 @@ static void clipper_machine_init(MachineClass *mc)
mc->max_cpus = 4;
mc->is_default = 1;
mc->default_cpu_type = ALPHA_CPU_TYPE_NAME("ev67");
+mc->default_ram_id = "ram";
}
DEFINE_MACHINE("clipper", clipper_machine_init)
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 179e1f7..1795e2f 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -58,7 +58,6 @@ typedef struct TyphoonState {
TyphoonCchip cchip;
TyphoonPchip pchip;
MemoryRegion dchip_region;
-MemoryRegion ram_region;
} TyphoonState;
/* Called when one of DRIR or DIM changes. */
@@ -817,8 +816,7 @@ static void typhoon_alarm_timer(void *opaque)
cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
}
-PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
- qemu_irq *p_rtc_irq,
+PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, qemu_irq *p_rtc_irq,
AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
{
MemoryRegion *addr_space = get_system_memory();
@@ -851,9 +849,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
/* Main memory region, 0x00... Real hardware supports 32GB,
but the address space hole reserved at this point is 8TB. */
-memory_region_allocate_system_memory(&s->ram_region, OBJECT(s), "ram",
- ram_size);
-memory_region_add_subregion(addr_space, 0, &s->ram_region);
+memory_region_add_subregion(addr_space, 0, ram);
/* TIGbus, 0x801.., 1GB. */
/* ??? The TIGbus is used for delivering interrupts, and access to
--
2.7.4
[PATCH v2 08/86] arm:aspeed: actually check RAM size
It's supposed that SOC will check if "-m" provided
RAM size is valid by setting "ram-size" property and
then board would read back valid (possibly corrected
value) to map RAM MemoryReging with valid size.
Well it isn't doing so, since check is called only
indirectly from
aspeed_sdmc_reset()->asc->compute_conf()
or much later when guest writes to configuration
register.
So depending on "-m" value QEMU end-ups with a warning
and an invalid MemoryRegion size allocated and mapped.
(examples:
-M ast2500-evb -m 1M
8000-00017ffe (prio 0, i/o): aspeed-ram-container
8000-800f (prio 0, ram): ram
8010-bfff (prio 0, i/o): max_ram
-M ast2500-evb -m 3G
8000-00017ffe (prio 0, i/o): aspeed-ram-container
8000-00013fff (prio 0, ram): ram
[DETECTED OVERFLOW!] 00014000-bfff (prio 0, i/o):
max_ram
)
On top of that sdmc falls back and reports to guest
"default" size, it thinks machine should have.
I don't know how hardware is supposed to work so
I've kept it as is.
But as for CLI side machine should honor whatever
user configured or error out to make user fix CLI.
This patch makes ram-size check actually work and
changes behavior from a warning later on during
machine reset to error_fatal at the moment SOC is
realized so user will have to fix RAM size on CLI
to start machine.
It also gets out of the way mutable ram-size logic,
so we could consolidate RAM allocation logic around
pre-allocated hostmem backend (supplied by user or
auto created by generic machine code depending on
supplied -m/mem-path/mem-prealloc options.
Signed-off-by: Igor Mammedov
---
CC: c...@kaod.org
CC: peter.mayd...@linaro.org
CC: and...@aj.id.au
CC: j...@jms.id.au
CC: qemu-...@nongnu.org
---
hw/arm/aspeed.c | 9 +
hw/misc/aspeed_sdmc.c | 5 +
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cc06af4..525c547 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -213,14 +213,7 @@ static void aspeed_machine_init(MachineState *machine)
"hw-prot-key", &error_abort);
}
object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
- &error_abort);
-
-/*
- * Allocate RAM after the memory controller has checked the size
- * was valid. If not, a default value is used.
- */
-ram_size = object_property_get_uint(OBJECT(&bmc->soc), "ram-size",
-&error_abort);
+ &error_fatal);
memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 3fc80f0..b398e36 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -165,6 +165,11 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error
**errp)
AspeedSDMCState *s = ASPEED_SDMC(dev);
AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
+if (!g_hash_table_contains(asc->ram2feat,
+ GINT_TO_POINTER(s->ram_size >> 20))) {
+error_setg(errp, "Invalid RAM size 0x%" PRIx64, s->ram_size);
+return;
+}
s->max_ram_size = asc->max_ram_size;
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_sdmc_ops, s,
--
2.7.4
[PATCH v2 12/86] arm:cubieboard: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
While at it, get rid of no longer needed CubieBoardState wrapper.
Signed-off-by: Igor Mammedov
---
CC: b.galv...@gmail.com
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/cubieboard.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 6dc2f1d..089f9a3 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -28,52 +28,42 @@ static struct arm_boot_info cubieboard_binfo = {
.board_id = 0x1008,
};
-typedef struct CubieBoardState {
-AwA10State *a10;
-MemoryRegion sdram;
-} CubieBoardState;
-
static void cubieboard_init(MachineState *machine)
{
-CubieBoardState *s = g_new(CubieBoardState, 1);
+AwA10State *a10 = AW_A10(object_new(TYPE_AW_A10));
Error *err = NULL;
-s->a10 = AW_A10(object_new(TYPE_AW_A10));
-
-object_property_set_int(OBJECT(&s->a10->emac), 1, "phy-addr", &err);
+object_property_set_int(OBJECT(&a10->emac), 1, "phy-addr", &err);
if (err != NULL) {
error_reportf_err(err, "Couldn't set phy address: ");
exit(1);
}
-object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
+object_property_set_int(OBJECT(&a10->timer), 32768, "clk0-freq", &err);
if (err != NULL) {
error_reportf_err(err, "Couldn't set clk0 frequency: ");
exit(1);
}
-object_property_set_int(OBJECT(&s->a10->timer), 2400, "clk1-freq",
-&err);
+object_property_set_int(OBJECT(&a10->timer), 2400, "clk1-freq", &err);
if (err != NULL) {
error_reportf_err(err, "Couldn't set clk1 frequency: ");
exit(1);
}
-object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
+object_property_set_bool(OBJECT(a10), true, "realized", &err);
if (err != NULL) {
error_reportf_err(err, "Couldn't realize Allwinner A10: ");
exit(1);
}
-memory_region_allocate_system_memory(&s->sdram, NULL, "cubieboard.ram",
- machine->ram_size);
memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
-&s->sdram);
+machine->ram);
/* TODO create and connect IDE devices for ide_drive_get() */
cubieboard_binfo.ram_size = machine->ram_size;
-arm_load_kernel(&s->a10->cpu, machine, &cubieboard_binfo);
+arm_load_kernel(&a10->cpu, machine, &cubieboard_binfo);
}
static void cubieboard_machine_init(MachineClass *mc)
@@ -84,6 +74,7 @@ static void cubieboard_machine_init(MachineClass *mc)
mc->block_default_type = IF_IDE;
mc->units_per_default_bus = 1;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_id = "cubieboard.ram";
}
DEFINE_MACHINE("cubieboard", cubieboard_machine_init)
--
2.7.4
[PATCH v2 05/86] initialize MachineState::ram in NUMA case
In case of NUMA there are 2 cases to consider:
1. '-numa node,memdev', the only one that will be available
for 5.0 and newer machine types.
In this case reuse current behavior, with only difference
memdevs are put into MachineState::ram container +
a temporary glue to keep memory_region_allocate_system_memory()
working until all boards converted.
2. fake NUMA ("-numa node mem" and default RAM splitting)
the later has been deprecated and is removed but the former
is going to stay available for compat reasons from 4.2 and
older machine types (libvirt was heavy user of this)
it takes allocate_system_memory_nonnuma() path, like non-NUMA
case and falls under conversion to memdev. So extend non-NUMA
MachineState::ram initialization introduced in previous patch
to take care of fake NUMA case.
Signed-off-by: Igor Mammedov
---
CC: ehabk...@redhat.com
CC: pbonz...@redhat.com
---
include/sysemu/numa.h | 1 +
hw/core/numa.c| 43 ++-
vl.c | 7 ---
3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index ba693cc..ad58ee8 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -112,5 +112,6 @@ void numa_default_auto_assign_ram(MachineClass *mc,
NodeInfo *nodes,
int nb_nodes, ram_addr_t size);
void numa_cpu_pre_plug(const struct CPUArchId *slot, DeviceState *dev,
Error **errp);
+bool numa_uses_legacy_mem(void);
#endif
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 8264336..e6baf2c 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -52,6 +52,11 @@ QemuOptsList qemu_numa_opts = {
};
static int have_memdevs;
+bool numa_uses_legacy_mem(void)
+{
+return !have_memdevs;
+}
+
static int have_mem;
static int max_numa_nodeid; /* Highest specified NUMA node ID, plus one.
* For all nodes, nodeid < max_numa_nodeid
@@ -652,6 +657,23 @@ void numa_default_auto_assign_ram(MachineClass *mc,
NodeInfo *nodes,
nodes[i].node_mem = size - usedmem;
}
+static void numa_init_memdev_container(MachineState *ms, MemoryRegion *ram)
+{
+int i;
+uint64_t addr = 0;
+
+for (i = 0; i < ms->numa_state->num_nodes; i++) {
+uint64_t size = ms->numa_state->nodes[i].node_mem;
+HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
+if (!backend) {
+continue;
+}
+MemoryRegion *seg = machine_consume_memdev(ms, backend);
+memory_region_add_subregion(ram, addr, seg);
+addr += size;
+}
+}
+
void numa_complete_configuration(MachineState *ms)
{
int i;
@@ -734,6 +756,12 @@ void numa_complete_configuration(MachineState *ms)
exit(1);
}
+if (!numa_uses_legacy_mem() && mc->default_ram_id) {
+ms->ram = g_new(MemoryRegion, 1);
+memory_region_init(ms->ram, OBJECT(ms), mc->default_ram_id,
+ ram_size);
+numa_init_memdev_container(ms, ms->ram);
+}
/* QEMU needs at least all unique node pair distances to build
* the whole NUMA distance table. QEMU treats the distance table
* as symmetric by default, i.e. distance A->B == distance B->A.
@@ -800,27 +828,16 @@ void memory_region_allocate_system_memory(MemoryRegion
*mr, Object *owner,
const char *name,
uint64_t ram_size)
{
-uint64_t addr = 0;
-int i;
MachineState *ms = MACHINE(qdev_get_machine());
if (ms->numa_state == NULL ||
-ms->numa_state->num_nodes == 0 || !have_memdevs) {
+ms->numa_state->num_nodes == 0 || numa_uses_legacy_mem()) {
allocate_system_memory_nonnuma(mr, owner, name, ram_size);
return;
}
memory_region_init(mr, owner, name, ram_size);
-for (i = 0; i < ms->numa_state->num_nodes; i++) {
-uint64_t size = ms->numa_state->nodes[i].node_mem;
-HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
-if (!backend) {
-continue;
-}
-MemoryRegion *seg = machine_consume_memdev(ms, backend);
-memory_region_add_subregion(mr, addr, seg);
-addr += size;
-}
+numa_init_memdev_container(ms, mr);
}
static void numa_stat_memory_devices(NumaNodeMem node_mem[])
diff --git a/vl.c b/vl.c
index d36a857..cf13987 100644
--- a/vl.c
+++ b/vl.c
@@ -4303,9 +4303,10 @@ int main(int argc, char **argv, char **envp)
}
parse_numa_opts(current_machine);
-if (!current_machine->ram_memdev &&
- machine_class->default_ram_size &&
- machine_class->default_ram_id) {
+if (numa_uses_legacy_mem() &&
+machine_class->default_ram_size &&
+machine_class->default_ram_id &&
+!current_machine->ram_memdev) {
create_d
[PATCH v2 09/86] hw:aspeed: drop warning and bogus ram_size fixup
It was useless to try fixup ram_size and print warning
on guest access to config register to begin with.
Now previous patch made sure that SDMC can not be realized
with invalid RAM size, so there is no case where warning
and not used ram_size fixup could be triggered.
So remove now dead code.
Signed-off-by: Igor Mammedov
---
CC: c...@kaod.org
CC: peter.mayd...@linaro.org
CC: and...@aj.id.au
CC: j...@jms.id.au
CC: qemu-...@nongnu.org
---
hw/misc/aspeed_sdmc.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index b398e36..942b27a 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -219,17 +219,8 @@ static int aspeed_get_ram_feat(AspeedSDMCState *s)
{
AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
int ram_mb = s->ram_size >> 20;
-gpointer val;
+gpointer val = g_hash_table_lookup(asc->ram2feat, GINT_TO_POINTER(ram_mb));
-if (g_hash_table_contains(asc->ram2feat, GINT_TO_POINTER(ram_mb))) {
-val = g_hash_table_lookup(asc->ram2feat, GINT_TO_POINTER(ram_mb));
-return GPOINTER_TO_INT(val);
-}
-
-warn_report("Invalid RAM size 0x%" PRIx64 ". Using default %dM",
- s->ram_size, asc->fallback_ram_size);
-s->ram_size = asc->fallback_ram_size << 20;
-val = g_hash_table_lookup(asc->ram2feat, &asc->fallback_ram_size);
return GPOINTER_TO_INT(val);
}
--
2.7.4
[PATCH v2 17/86] arm:integratorcp: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: peter.ch...@nicta.com.au
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/integratorcp.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 5249708..89c223b 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
Object *cpuobj;
ARMCPU *cpu;
MemoryRegion *address_space_mem = get_system_memory();
-MemoryRegion *ram = g_new(MemoryRegion, 1);
MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
qemu_irq pic[32];
DeviceState *dev, *sic, *icp;
@@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
cpu = ARM_CPU(cpuobj);
-memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
- ram_size);
/* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */
/* ??? RAM should repeat to fill physical memory space. */
/* SDRAM at address zero*/
-memory_region_add_subregion(address_space_mem, 0, ram);
+memory_region_add_subregion(address_space_mem, 0, machine->ram);
/* And again at address 0x8000 */
-memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
+memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
+ 0, ram_size);
memory_region_add_subregion(address_space_mem, 0x8000, ram_alias);
dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
@@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
mc->init = integratorcp_init;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+mc->default_ram_id = "integrator.ram";
}
DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
--
2.7.4
[PATCH v2 13/86] arm:digic_boards: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
remove no longer needed DigicBoardState
PS2:
while at it add check for user supplied RAM size and error
out if it mismatches board expected value.
Signed-off-by: Igor Mammedov
---
v2:
* fix format string causing build failure on 32-bit host
(Philippe Mathieu-Daudé )
CC: antonynpav...@gmail.com
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/digic_boards.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index ef3fc2b..9f094d4 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -35,39 +35,38 @@
#include "hw/loader.h"
#include "sysemu/sysemu.h"
#include "sysemu/qtest.h"
+#include "qemu/units.h"
#define DIGIC4_ROM0_BASE 0xf000
#define DIGIC4_ROM1_BASE 0xf800
#define DIGIC4_ROM_MAX_SIZE 0x0800
-typedef struct DigicBoardState {
-DigicState *digic;
-MemoryRegion ram;
-} DigicBoardState;
-
typedef struct DigicBoard {
-hwaddr ram_size;
-void (*add_rom0)(DigicBoardState *, hwaddr, const char *);
+void (*add_rom0)(DigicState *, hwaddr, const char *);
const char *rom0_def_filename;
-void (*add_rom1)(DigicBoardState *, hwaddr, const char *);
+void (*add_rom1)(DigicState *, hwaddr, const char *);
const char *rom1_def_filename;
} DigicBoard;
-static void digic4_board_init(DigicBoard *board)
+static void digic4_board_init(MachineState *machine, DigicBoard *board)
{
Error *err = NULL;
+MachineClass *mc = MACHINE_GET_CLASS(machine);
-DigicBoardState *s = g_new(DigicBoardState, 1);
+if (machine->ram_size != mc->default_ram_size) {
+error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
+ mc->default_ram_size);
+exit(EXIT_FAILURE);
+}
-s->digic = DIGIC(object_new(TYPE_DIGIC));
-object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
+DigicState *s = DIGIC(object_new(TYPE_DIGIC));
+object_property_set_bool(OBJECT(s), true, "realized", &err);
if (err != NULL) {
error_reportf_err(err, "Couldn't realize DIGIC SoC: ");
exit(1);
}
-memory_region_allocate_system_memory(&s->ram, NULL, "ram",
board->ram_size);
-memory_region_add_subregion(get_system_memory(), 0, &s->ram);
+memory_region_add_subregion(get_system_memory(), 0, machine->ram);
if (board->add_rom0) {
board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
@@ -78,7 +77,7 @@ static void digic4_board_init(DigicBoard *board)
}
}
-static void digic_load_rom(DigicBoardState *s, hwaddr addr,
+static void digic_load_rom(DigicState *s, hwaddr addr,
hwaddr max_size, const char *def_filename)
{
target_long rom_size;
@@ -118,7 +117,7 @@ static void digic_load_rom(DigicBoardState *s, hwaddr addr,
* Samsung K8P3215UQB
* 64M Bit (4Mx16) Page Mode / Multi-Bank NOR Flash Memory
*/
-static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
+static void digic4_add_k8p3215uqb_rom(DigicState *s, hwaddr addr,
const char *def_filename)
{
#define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
@@ -135,14 +134,13 @@ static void digic4_add_k8p3215uqb_rom(DigicBoardState *s,
hwaddr addr,
}
static DigicBoard digic4_board_canon_a1100 = {
-.ram_size = 64 * 1024 * 1024,
.add_rom1 = digic4_add_k8p3215uqb_rom,
.rom1_def_filename = "canon-a1100-rom1.bin",
};
static void canon_a1100_init(MachineState *machine)
{
-digic4_board_init(&digic4_board_canon_a1100);
+digic4_board_init(machine, &digic4_board_canon_a1100);
}
static void canon_a1100_machine_init(MachineClass *mc)
@@ -150,6 +148,8 @@ static void canon_a1100_machine_init(MachineClass *mc)
mc->desc = "Canon PowerShot A1100 IS";
mc->init = &canon_a1100_init;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_size = 64 * MiB;
+mc->default_ram_id = "ram";
}
DEFINE_MACHINE("canon-a1100", canon_a1100_machine_init)
--
2.7.4
[PATCH v2 10/86] arm:aspeed: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: c...@kaod.org
CC: peter.mayd...@linaro.org
CC: and...@aj.id.au
CC: j...@jms.id.au
CC: qemu-...@nongnu.org
---
hw/arm/aspeed.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 525c547..330254b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -35,7 +35,6 @@ static struct arm_boot_info aspeed_board_binfo = {
struct AspeedBoardState {
AspeedSoCState soc;
MemoryRegion ram_container;
-MemoryRegion ram;
MemoryRegion max_ram;
};
@@ -184,6 +183,7 @@ static void aspeed_machine_init(MachineState *machine)
memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
UINT32_MAX);
+memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
object_initialize_child(OBJECT(machine), "soc", &bmc->soc,
(sizeof(bmc->soc)), amc->soc_name, &error_abort,
@@ -215,8 +215,6 @@ static void aspeed_machine_init(MachineState *machine)
object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
&error_fatal);
-memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
-memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
memory_region_add_subregion(get_system_memory(),
sc->memmap[ASPEED_SDRAM],
&bmc->ram_container);
@@ -393,6 +391,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, void
*data)
mc->no_floppy = 1;
mc->no_cdrom = 1;
mc->no_parallel = 1;
+mc->default_ram_id = "ram";
}
static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
--
2.7.4
[PATCH v2 11/86] arm:collie: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
- while at it add check for user supplied RAM size and error
out if it mismatches board expected value.
- introduce RAM_ADDR_UFMT to avoid build errors on 32-bit hosts
when specifying format string for ram_addr_t type
Signed-off-by: Igor Mammedov
---
v2:
* fix format string causing build failure on 32-bit host
(Philippe Mathieu-Daudé )
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
include/exec/cpu-common.h | 2 ++
hw/arm/collie.c | 15 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 81753bb..0ea4886 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -33,10 +33,12 @@ enum device_endian {
typedef uint64_t ram_addr_t;
# define RAM_ADDR_MAX UINT64_MAX
# define RAM_ADDR_FMT "%" PRIx64
+# define RAM_ADDR_UFMT "%" PRIu64
#else
typedef uintptr_t ram_addr_t;
# define RAM_ADDR_MAX UINTPTR_MAX
# define RAM_ADDR_FMT "%" PRIxPTR
+# define RAM_ADDR_UFMT "%" PRIuPTR
#endif
extern ram_addr_t ram_size;
diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 970a440..176cf09 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -20,20 +20,23 @@
static struct arm_boot_info collie_binfo = {
.loader_start = SA_SDCS0,
-.ram_size = 0x2000,
};
static void collie_init(MachineState *machine)
{
StrongARMState *s;
DriveInfo *dinfo;
-MemoryRegion *sdram = g_new(MemoryRegion, 1);
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+if (machine->ram_size != mc->default_ram_size) {
+error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
+ mc->default_ram_size);
+exit(EXIT_FAILURE);
+}
s = sa1110_init(machine->cpu_type);
-memory_region_allocate_system_memory(sdram, NULL, "strongarm.sdram",
- collie_binfo.ram_size);
-memory_region_add_subregion(get_system_memory(), SA_SDCS0, sdram);
+memory_region_add_subregion(get_system_memory(), SA_SDCS0, machine->ram);
dinfo = drive_get(IF_PFLASH, 0, 0);
pflash_cfi01_register(SA_CS0, "collie.fl1", 0x0200,
@@ -57,6 +60,8 @@ static void collie_machine_init(MachineClass *mc)
mc->init = collie_init;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("sa1110");
+mc->default_ram_size = 0x2000;
+mc->default_ram_id = "strongarm.sdram";
}
DEFINE_MACHINE("collie", collie_machine_init)
--
2.7.4
[PATCH v2 19/86] arm:kzm: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
Reviewed-by: Philippe Mathieu-Daudé
---
CC: peter.ch...@nicta.com.au
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/kzm.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 27800c5..f63902e 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -51,7 +51,6 @@
typedef struct IMX31KZM {
FslIMX31State soc;
-MemoryRegion ram;
MemoryRegion ram_alias;
} IMX31KZM;
@@ -84,10 +83,8 @@ static void kzm_init(MachineState *machine)
exit(EXIT_FAILURE);
}
-memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
- machine->ram_size);
memory_region_add_subregion(get_system_memory(), FSL_IMX31_SDRAM0_ADDR,
-&s->ram);
+machine->ram);
/* initialize the alias memory if any */
for (i = 0, ram_size = machine->ram_size, alias_offset = 0;
@@ -107,7 +104,8 @@ static void kzm_init(MachineState *machine)
if (size < ram[i].size) {
memory_region_init_alias(&s->ram_alias, NULL, "ram.alias",
- &s->ram, alias_offset, ram[i].size -
size);
+ machine->ram,
+ alias_offset, ram[i].size - size);
memory_region_add_subregion(get_system_memory(),
ram[i].addr + size, &s->ram_alias);
}
@@ -139,6 +137,7 @@ static void kzm_machine_init(MachineClass *mc)
mc->desc = "ARM KZM Emulation Baseboard (ARM1136)";
mc->init = kzm_init;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_id = "kzm.ram";
}
DEFINE_MACHINE("kzm", kzm_machine_init)
--
2.7.4
[PATCH v2 18/86] arm:kzm: drop RAM size fixup
If user provided non-sense RAM size, board will complain and
continue running with max RAM size supported.
Also RAM is going to be allocated by generic code, so it won't be
possible for board to fix things up for user.
Make it error message and exit to force user fix CLI,
instead of accepting non-sense CLI values.
Signed-off-by: Igor Mammedov
---
CC: peter.ch...@nicta.com.au
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/kzm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 1d5ef28..27800c5 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -78,10 +78,10 @@ static void kzm_init(MachineState *machine)
/* Check the amount of memory is compatible with the SOC */
if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
-warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
+error_report("RAM size " RAM_ADDR_FMT " above max supported, "
"reduced to %x", machine->ram_size,
FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
-machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
+exit(EXIT_FAILURE);
}
memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
--
2.7.4
[PATCH v2 15/86] arm:imx25_pdk: drop RAM size fixup
If user provided non-sense RAM size, board will complain and
continue running with max RAM size supported.
Also RAM is going to be allocated by generic code, so it won't be
possible for board to fix things up for user.
Make it error message and exit to force user fix CLI,
instead of accepting non-sense CLI values.
Signed-off-by: Igor Mammedov
---
CC: j...@tribudubois.net
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/imx25_pdk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index c76fc2b..a2b7b35 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -78,10 +78,10 @@ static void imx25_pdk_init(MachineState *machine)
/* We need to initialize our memory */
if (machine->ram_size > (FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE)) {
-warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
+error_report("RAM size " RAM_ADDR_FMT " above max supported, "
"reduced to %x", machine->ram_size,
FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE);
-machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
+exit(EXIT_FAILURE);
}
memory_region_allocate_system_memory(&s->ram, NULL, "imx25.ram",
--
2.7.4
[PATCH v2 14/86] arm:highbank: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: r...@kernel.org
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/highbank.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 518d935..ac9de94 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -236,7 +236,6 @@ enum cxmachines {
*/
static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
{
-ram_addr_t ram_size = machine->ram_size;
DeviceState *dev = NULL;
SysBusDevice *busdev;
qemu_irq pic[128];
@@ -247,7 +246,6 @@ static void calxeda_init(MachineState *machine, enum
cxmachines machine_id)
qemu_irq cpu_virq[4];
qemu_irq cpu_vfiq[4];
MemoryRegion *sysram;
-MemoryRegion *dram;
MemoryRegion *sysmem;
char *sysboot_filename;
@@ -290,10 +288,8 @@ static void calxeda_init(MachineState *machine, enum
cxmachines machine_id)
}
sysmem = get_system_memory();
-dram = g_new(MemoryRegion, 1);
-memory_region_allocate_system_memory(dram, NULL, "highbank.dram",
ram_size);
/* SDRAM at address zero. */
-memory_region_add_subregion(sysmem, 0, dram);
+memory_region_add_subregion(sysmem, 0, machine->ram);
sysram = g_new(MemoryRegion, 1);
memory_region_init_ram(sysram, NULL, "highbank.sysram", 0x8000,
@@ -387,7 +383,7 @@ static void calxeda_init(MachineState *machine, enum
cxmachines machine_id)
/* TODO create and connect IDE devices for ide_drive_get() */
-highbank_binfo.ram_size = ram_size;
+highbank_binfo.ram_size = machine->ram_size;
/* highbank requires a dtb in order to boot, and the dtb will override
* the board ID. The following value is ignored, so set it to -1 to be
* clear that the value is meaningless.
@@ -430,6 +426,7 @@ static void highbank_class_init(ObjectClass *oc, void *data)
mc->units_per_default_bus = 1;
mc->max_cpus = 4;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_id = "highbank.dram";
}
static const TypeInfo highbank_type = {
@@ -448,6 +445,7 @@ static void midway_class_init(ObjectClass *oc, void *data)
mc->units_per_default_bus = 1;
mc->max_cpus = 4;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_id = "highbank.dram";
}
static const TypeInfo midway_type = {
--
2.7.4
[PATCH v2 20/86] arm:mcimx6ul-evk: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
remove no longer needed MCIMX6ULEVK
Signed-off-by: Igor Mammedov
---
CC: j...@tribudubois.net
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/mcimx6ul-evk.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index e90b393..23a71ed 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -19,15 +19,10 @@
#include "qemu/error-report.h"
#include "sysemu/qtest.h"
-typedef struct {
-FslIMX6ULState soc;
-MemoryRegion ram;
-} MCIMX6ULEVK;
-
static void mcimx6ul_evk_init(MachineState *machine)
{
static struct arm_boot_info boot_info;
-MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
+FslIMX6ULState *s;
int i;
if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
@@ -43,15 +38,12 @@ static void mcimx6ul_evk_init(MachineState *machine)
.nb_cpus = machine->smp.cpus,
};
-object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
-TYPE_FSL_IMX6UL, &error_fatal, NULL);
-
-object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
+s = FSL_IMX6UL(object_new(TYPE_FSL_IMX6UL));
+object_property_add_child(OBJECT(machine), "soc", OBJECT(s), &error_fatal);
+object_property_set_bool(OBJECT(s), true, "realized", &error_fatal);
-memory_region_allocate_system_memory(&s->ram, NULL, "mcimx6ul-evk.ram",
- machine->ram_size);
-memory_region_add_subregion(get_system_memory(),
-FSL_IMX6UL_MMDC_ADDR, &s->ram);
+memory_region_add_subregion(get_system_memory(), FSL_IMX6UL_MMDC_ADDR,
+machine->ram);
for (i = 0; i < FSL_IMX6UL_NUM_USDHCS; i++) {
BusState *bus;
@@ -61,7 +53,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
di = drive_get_next(IF_SD);
blk = di ? blk_by_legacy_dinfo(di) : NULL;
-bus = qdev_get_child_bus(DEVICE(&s->soc.usdhc[i]), "sd-bus");
+bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus");
carddev = qdev_create(bus, TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
object_property_set_bool(OBJECT(carddev), true,
@@ -69,7 +61,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
}
if (!qtest_enabled()) {
-arm_load_kernel(&s->soc.cpu, machine, &boot_info);
+arm_load_kernel(&s->cpu, machine, &boot_info);
}
}
@@ -78,5 +70,6 @@ static void mcimx6ul_evk_machine_init(MachineClass *mc)
mc->desc = "Freescale i.MX6UL Evaluation Kit (Cortex A7)";
mc->init = mcimx6ul_evk_init;
mc->max_cpus = FSL_IMX6UL_NUM_CPUS;
+mc->default_ram_id = "mcimx6ul-evk.ram";
}
DEFINE_MACHINE("mcimx6ul-evk", mcimx6ul_evk_machine_init)
--
2.7.4
[PATCH v2 34/86] arm:xilinx_zynq: drop RAM size fixup
If user provided non-sense RAM size, board will complain and
continue running with max RAM size supported.
Also RAM is going to be allocated by generic code, so it won't be
possible for board to fix things up for user.
Make it error message and exit to force user fix CLI,
instead of accepting non-sense CLI values.
Signed-off-by: Igor Mammedov
---
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
CC: edgar.igles...@gmail.com
CC: alist...@alistair23.me
---
hw/arm/xilinx_zynq.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3a0fa5b..df950fc 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -158,7 +158,6 @@ static inline void zynq_init_spi_flashes(uint32_t
base_addr, qemu_irq irq,
static void zynq_init(MachineState *machine)
{
-ram_addr_t ram_size = machine->ram_size;
ARMCPU *cpu;
MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
@@ -168,6 +167,12 @@ static void zynq_init(MachineState *machine)
qemu_irq pic[64];
int n;
+/* max 2GB ram */
+if (machine->ram_size > 0x8000) {
+error_report("RAM size more than %d is not supported", 0x8000);
+exit(EXIT_FAILURE);
+}
+
cpu = ARM_CPU(object_new(machine->cpu_type));
/* By default A9 CPUs have EL3 enabled. This board does not
@@ -184,14 +189,9 @@ static void zynq_init(MachineState *machine)
&error_fatal);
object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
-/* max 2GB ram */
-if (ram_size > 0x8000) {
-ram_size = 0x8000;
-}
-
/* DDR remapped to address zero. */
memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
- ram_size);
+ machine->ram_size);
memory_region_add_subregion(address_space_mem, 0, ext_ram);
/* 256K of on-chip memory */
@@ -300,7 +300,7 @@ static void zynq_init(MachineState *machine)
sysbus_connect_irq(busdev, 0, pic[40 - IRQ_OFFSET]);
sysbus_mmio_map(busdev, 0, 0xF8007000);
-zynq_binfo.ram_size = ram_size;
+zynq_binfo.ram_size = machine->ram_size;
zynq_binfo.nb_cpus = 1;
zynq_binfo.board_id = 0xd32;
zynq_binfo.loader_start = 0;
--
2.7.4
[PATCH v2 23/86] arm:mps2: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
while at it add check for user supplied RAM size and error
out if it mismatches board expected value.
Signed-off-by: Igor Mammedov
---
v2:
* fix format string causing build failure on 32-bit host
(Philippe Mathieu-Daudé )
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/mps2.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index d002b12..5867ff0 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -55,7 +55,6 @@ typedef struct {
MachineState parent;
ARMv7MState armv7m;
-MemoryRegion psram;
MemoryRegion ssram1;
MemoryRegion ssram1_m;
MemoryRegion ssram23;
@@ -118,6 +117,12 @@ static void mps2_common_init(MachineState *machine)
exit(1);
}
+if (machine->ram_size != mc->default_ram_size) {
+error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
+ mc->default_ram_size);
+exit(EXIT_FAILURE);
+}
+
/* The FPGA images have an odd combination of different RAMs,
* because in hardware they are different implementations and
* connected to different buses, giving varying performance/size
@@ -146,9 +151,7 @@ static void mps2_common_init(MachineState *machine)
* This is of no use for QEMU so we don't implement it (as if
* zbt_boot_ctrl is always zero).
*/
-memory_region_allocate_system_memory(&mms->psram,
- NULL, "mps.ram", 16 * MiB);
-memory_region_add_subregion(system_memory, 0x2100, &mms->psram);
+memory_region_add_subregion(system_memory, 0x2100, machine->ram);
switch (mmc->fpga_type) {
case FPGA_AN385:
@@ -338,6 +341,8 @@ static void mps2_class_init(ObjectClass *oc, void *data)
mc->init = mps2_common_init;
mc->max_cpus = 1;
+mc->default_ram_size = 16 * MiB;
+mc->default_ram_id = "mps.ram";
}
static void mps2_an385_class_init(ObjectClass *oc, void *data)
--
2.7.4
[PATCH v2 16/86] arm:imx25_pdk: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: j...@tribudubois.net
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/imx25_pdk.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index a2b7b35..9087fcb 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -58,7 +58,6 @@
typedef struct IMX25PDK {
FslIMX25State soc;
-MemoryRegion ram;
MemoryRegion ram_alias;
} IMX25PDK;
@@ -84,10 +83,8 @@ static void imx25_pdk_init(MachineState *machine)
exit(EXIT_FAILURE);
}
-memory_region_allocate_system_memory(&s->ram, NULL, "imx25.ram",
- machine->ram_size);
memory_region_add_subregion(get_system_memory(), FSL_IMX25_SDRAM0_ADDR,
-&s->ram);
+machine->ram);
/* initialize the alias memory if any */
for (i = 0, ram_size = machine->ram_size, alias_offset = 0;
@@ -107,7 +104,8 @@ static void imx25_pdk_init(MachineState *machine)
if (size < ram[i].size) {
memory_region_init_alias(&s->ram_alias, NULL, "ram.alias",
- &s->ram, alias_offset, ram[i].size -
size);
+ machine->ram,
+ alias_offset, ram[i].size - size);
memory_region_add_subregion(get_system_memory(),
ram[i].addr + size, &s->ram_alias);
}
@@ -135,6 +133,7 @@ static void imx25_pdk_machine_init(MachineClass *mc)
mc->desc = "ARM i.MX25 PDK board (ARM926)";
mc->init = imx25_pdk_init;
mc->ignore_memory_transaction_failures = true;
+mc->default_ram_id = "imx25.ram";
}
DEFINE_MACHINE("imx25-pdk", imx25_pdk_machine_init)
--
2.7.4
[PATCH v2 24/86] arm:musicpal: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
while at it add check for user supplied RAM size and error
out if it mismatches board expected value.
Signed-off-by: Igor Mammedov
---
v2:
* fix format string causing build failure on 32-bit host
(Philippe Mathieu-Daudé )
CC: jan.kis...@web.de
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/musicpal.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index f68a399..4242c4a 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1589,16 +1589,20 @@ static void musicpal_init(MachineState *machine)
int i;
unsigned long flash_size;
DriveInfo *dinfo;
+MachineClass *mc = MACHINE_GET_CLASS(machine);
MemoryRegion *address_space_mem = get_system_memory();
-MemoryRegion *ram = g_new(MemoryRegion, 1);
MemoryRegion *sram = g_new(MemoryRegion, 1);
+/* For now we use a fixed - the original - RAM size */
+if (machine->ram_size != mc->default_ram_size) {
+error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
+ mc->default_ram_size);
+exit(EXIT_FAILURE);
+}
+
cpu = ARM_CPU(cpu_create(machine->cpu_type));
-/* For now we use a fixed - the original - RAM size */
-memory_region_allocate_system_memory(ram, NULL, "musicpal.ram",
- MP_RAM_DEFAULT_SIZE);
-memory_region_add_subregion(address_space_mem, 0, ram);
+memory_region_add_subregion(address_space_mem, 0, machine->ram);
memory_region_init_ram(sram, NULL, "musicpal.sram", MP_SRAM_SIZE,
&error_fatal);
@@ -1714,6 +1718,8 @@ static void musicpal_machine_init(MachineClass *mc)
mc->init = musicpal_init;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+mc->default_ram_size = MP_RAM_DEFAULT_SIZE;
+mc->default_ram_id = "musicpal.ram";
}
DEFINE_MACHINE("musicpal", musicpal_machine_init)
--
2.7.4
[PATCH v2 35/86] arm:xilinx_zynq: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
CC: edgar.igles...@gmail.com
CC: alist...@alistair23.me
---
hw/arm/xilinx_zynq.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index df950fc..0ef9688 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -160,7 +160,6 @@ static void zynq_init(MachineState *machine)
{
ARMCPU *cpu;
MemoryRegion *address_space_mem = get_system_memory();
-MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
DeviceState *dev;
SysBusDevice *busdev;
@@ -190,9 +189,7 @@ static void zynq_init(MachineState *machine)
object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
/* DDR remapped to address zero. */
-memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
- machine->ram_size);
-memory_region_add_subregion(address_space_mem, 0, ext_ram);
+memory_region_add_subregion(address_space_mem, 0, machine->ram);
/* 256K of on-chip memory */
memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 * KiB,
@@ -318,6 +315,7 @@ static void zynq_machine_init(MachineClass *mc)
mc->no_sdcard = 1;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+mc->default_ram_id = "zynq.ext_ram";
}
DEFINE_MACHINE("xilinx-zynq-a9", zynq_machine_init)
--
2.7.4
[PATCH v2 21/86] arm:mcimx7d-sabre: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
remove no longer needed MCIMX7Sabre
Signed-off-by: Igor Mammedov
---
CC: andrew.smir...@gmail.com
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/mcimx7d-sabre.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 0d1f62d..de1e264 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -21,15 +21,10 @@
#include "qemu/error-report.h"
#include "sysemu/qtest.h"
-typedef struct {
-FslIMX7State soc;
-MemoryRegion ram;
-} MCIMX7Sabre;
-
static void mcimx7d_sabre_init(MachineState *machine)
{
static struct arm_boot_info boot_info;
-MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
+FslIMX7State *s;
int i;
if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
@@ -45,15 +40,12 @@ static void mcimx7d_sabre_init(MachineState *machine)
.nb_cpus = machine->smp.cpus,
};
-object_initialize_child(OBJECT(machine), "soc",
-&s->soc, sizeof(s->soc),
-TYPE_FSL_IMX7, &error_fatal, NULL);
-object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
+s = FSL_IMX7(object_new(TYPE_FSL_IMX7));
+object_property_add_child(OBJECT(machine), "soc", OBJECT(s), &error_fatal);
+object_property_set_bool(OBJECT(s), true, "realized", &error_fatal);
-memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram",
- machine->ram_size);
-memory_region_add_subregion(get_system_memory(),
-FSL_IMX7_MMDC_ADDR, &s->ram);
+memory_region_add_subregion(get_system_memory(), FSL_IMX7_MMDC_ADDR,
+machine->ram);
for (i = 0; i < FSL_IMX7_NUM_USDHCS; i++) {
BusState *bus;
@@ -63,7 +55,7 @@ static void mcimx7d_sabre_init(MachineState *machine)
di = drive_get_next(IF_SD);
blk = di ? blk_by_legacy_dinfo(di) : NULL;
-bus = qdev_get_child_bus(DEVICE(&s->soc.usdhc[i]), "sd-bus");
+bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus");
carddev = qdev_create(bus, TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
object_property_set_bool(OBJECT(carddev), true,
@@ -71,7 +63,7 @@ static void mcimx7d_sabre_init(MachineState *machine)
}
if (!qtest_enabled()) {
-arm_load_kernel(&s->soc.cpu[0], machine, &boot_info);
+arm_load_kernel(&s->cpu[0], machine, &boot_info);
}
}
@@ -80,5 +72,6 @@ static void mcimx7d_sabre_machine_init(MachineClass *mc)
mc->desc = "Freescale i.MX7 DUAL SABRE (Cortex A7)";
mc->init = mcimx7d_sabre_init;
mc->max_cpus = FSL_IMX7_NUM_CPUS;
+mc->default_ram_id = "mcimx7d-sabre.ram";
}
DEFINE_MACHINE("mcimx7d-sabre", mcimx7d_sabre_machine_init)
--
2.7.4
[PATCH v2 31/86] arm:versatilepb: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
Signed-off-by: Igor Mammedov
---
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/versatilepb.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index e86af01..f3c4a50 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -184,7 +184,6 @@ static void versatile_init(MachineState *machine, int
board_id)
Object *cpuobj;
ARMCPU *cpu;
MemoryRegion *sysmem = get_system_memory();
-MemoryRegion *ram = g_new(MemoryRegion, 1);
qemu_irq pic[32];
qemu_irq sic[32];
DeviceState *dev, *sysctl;
@@ -220,11 +219,9 @@ static void versatile_init(MachineState *machine, int
board_id)
cpu = ARM_CPU(cpuobj);
-memory_region_allocate_system_memory(ram, NULL, "versatile.ram",
- machine->ram_size);
/* ??? RAM should repeat to fill physical memory space. */
/* SDRAM at address zero. */
-memory_region_add_subregion(sysmem, 0, ram);
+memory_region_add_subregion(sysmem, 0, machine->ram);
sysctl = qdev_create(NULL, "realview_sysctl");
qdev_prop_set_uint32(sysctl, "sys_id", 0x41007004);
@@ -398,6 +395,7 @@ static void versatilepb_class_init(ObjectClass *oc, void
*data)
mc->block_default_type = IF_SCSI;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+mc->default_ram_id = "versatile.ram";
}
static const TypeInfo versatilepb_type = {
@@ -415,6 +413,7 @@ static void versatileab_class_init(ObjectClass *oc, void
*data)
mc->block_default_type = IF_SCSI;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+mc->default_ram_id = "versatile.ram";
}
static const TypeInfo versatileab_type = {
--
2.7.4
[PATCH v2 27/86] arm:palm: use memdev for RAM
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.
PS:
while at it add check for user supplied RAM size and error
out if it mismatches board expected value.
Signed-off-by: Igor Mammedov
---
v2:
* fix format string causing build failure on 32-bit host
(Philippe Mathieu-Daudé )
CC: balr...@gmail.com
CC: peter.mayd...@linaro.org
CC: qemu-...@nongnu.org
---
hw/arm/palm.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 72eca8c..336ac4b 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -181,7 +181,6 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu)
static struct arm_boot_info palmte_binfo = {
.loader_start = OMAP_EMIFF_BASE,
-.ram_size = 0x0200,
.board_id = 0x331,
};
@@ -195,15 +194,20 @@ static void palmte_init(MachineState *machine)
static uint32_t cs2val = 0xe1a0;
static uint32_t cs3val = 0xe1a0e1a0;
int rom_size, rom_loaded = 0;
-MemoryRegion *dram = g_new(MemoryRegion, 1);
+MachineClass *mc = MACHINE_GET_CLASS(machine);
MemoryRegion *flash = g_new(MemoryRegion, 1);
MemoryRegion *cs = g_new(MemoryRegion, 4);
-memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
- palmte_binfo.ram_size);
-memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram);
+if (machine->ram_size != mc->default_ram_size) {
+error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
+ mc->default_ram_size);
+exit(EXIT_FAILURE);
+}
+
+memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE,
+machine->ram);
-mpu = omap310_mpu_init(dram, machine->cpu_type);
+mpu = omap310_mpu_init(machine->ram, machine->cpu_type);
/* External Flash (EMIFS) */
memory_region_init_ram(flash, NULL, "palmte.flash", flash_size,
@@ -265,6 +269,8 @@ static void palmte_machine_init(MachineClass *mc)
mc->init = palmte_init;
mc->ignore_memory_transaction_failures = true;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("ti925t");
+mc->default_ram_size = 0x0200;
+mc->default_ram_id = "omap1.dram";
}
DEFINE_MACHINE("cheetah", palmte_machine_init)
--
2.7.4
