From: Laszlo Ersek <lersek@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Marcel Apfelbaum <marcel.a@redhat.com>,
Jan Kiszka <jan.kiszka@siemens.com>,
"edk2-devel@lists.sourceforge.net"
<edk2-devel@lists.sourceforge.net>,
qemu-devel <qemu-devel@nongnu.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
Anthony Liguori <anthony@codemonkey.ws>,
Paolo Bonzini <pbonzini@redhat.com>,
Jordan Justen <jljusten@gmail.com>, afaerber <afaerber@suse.de>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 1/2] i386/pc: propagate flash size from pc_system_flash_init() to pc_init1()
Date: Fri, 08 Nov 2013 16:27:57 +0100 [thread overview]
Message-ID: <527D02FD.2000109@redhat.com> (raw)
In-Reply-To: <CAFEAcA_o1ZsLX8dtW5L+3OQgtntix2tfqZweb9r7GdW=P6ZYjw@mail.gmail.com>
On 11/08/13 16:16, Peter Maydell wrote:
> On 8 November 2013 15:07, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 11/08/13 07:09, Jordan Justen wrote:
>>> int64_t? :)
>>
>> Heh, yes, I did cringe when I wrote that, but if you check the
>> bottom-most function, where the assignment happens,
>> pc_system_flash_init(), it declares the local "size" variable as
>> int64_t. I've given up on arguing for sensible unsigned types so I just
>> went with the flow
>
> That's a bug in that function which should be fixed.
> This is a memory region size and those are uint64_t.
>
> That said, having to pass the size of a sub-sub-region
> all the way back up the call stack is very odd and makes
> me wonder if it's really the right way to do it...
> The top level shouldn't have to care like that about
> details of the bottom of the callstack.
I agree. It's just that system.flash and pci-hole are siblings in the
same container (they shouldn't overlap, *or* they should have clearly
different priorities between them). We have two call chains rooted in
pc_init1(), and the "ends" of those chains need to coordinate with each
other (they set up the two regions, respectively, and both need the
boundary between them). We could introduce a new global, but that's not
exactly a step forward :)
Thanks
Laszlo
next prev parent reply other threads:[~2013-11-08 15:25 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-03 20:48 [Qemu-devel] [PATCH] exec: fix regression by making system-memory region UINT64_MAX size Marcel Apfelbaum
2013-11-03 21:26 ` Peter Maydell
2013-11-04 6:18 ` Michael S. Tsirkin
2013-11-04 9:33 ` Marcel Apfelbaum
2013-11-04 9:59 ` Michael S. Tsirkin
2013-11-07 20:27 ` Jordan Justen
2013-11-07 20:44 ` Marcel Apfelbaum
2013-11-07 21:12 ` Laszlo Ersek
2013-11-07 21:21 ` [Qemu-devel] [edk2] " Paolo Bonzini
2013-11-07 21:32 ` Laszlo Ersek
2013-11-07 21:24 ` [Qemu-devel] " Marcel Apfelbaum
2013-11-07 21:31 ` Paolo Bonzini
2013-11-07 21:38 ` Marcel Apfelbaum
2013-11-07 21:51 ` Peter Maydell
2013-11-07 22:06 ` Marcel Apfelbaum
2013-11-08 8:05 ` Paolo Bonzini
2013-11-08 10:44 ` Peter Maydell
2013-11-08 11:00 ` Paolo Bonzini
2013-11-08 15:16 ` Marcel Apfelbaum
2013-11-08 15:08 ` Marcel Apfelbaum
2013-11-08 16:12 ` Paolo Bonzini
2013-11-08 16:19 ` Marcel Apfelbaum
2013-11-07 21:48 ` Laszlo Ersek
2013-11-07 22:09 ` Marcel Apfelbaum
2013-11-07 22:23 ` [Qemu-devel] [PATCH 0/2] " Laszlo Ersek
2013-11-07 22:23 ` [Qemu-devel] [PATCH 1/2] i386/pc: propagate flash size from pc_system_flash_init() to pc_init1() Laszlo Ersek
2013-11-08 6:09 ` Jordan Justen
2013-11-08 15:07 ` Laszlo Ersek
2013-11-08 15:16 ` Peter Maydell
2013-11-08 15:27 ` Laszlo Ersek [this message]
2013-11-08 15:28 ` Peter Maydell
2013-11-07 22:23 ` [Qemu-devel] [PATCH 2/2] i386/pc_piix: the pci-hole should end where the system flash starts Laszlo Ersek
2013-11-08 10:14 ` [Qemu-devel] reverting commit a53ae8e934cd54686875b5bcfc2f434244ee55d6 Re: [edk2] [PATCH 0/2] Re: exec: fix regression by making system-memory region UINT64_MAX size Paolo Bonzini
2013-11-08 16:37 ` [Qemu-devel] " Igor Mammedov
2013-11-08 15:42 ` [Qemu-devel] [PATCH] " Andreas Färber
2013-11-08 16:19 ` Laszlo Ersek
2013-11-08 16:26 ` Paolo Bonzini
2013-11-08 17:09 ` Andreas Färber
2013-11-08 17:15 ` Paolo Bonzini
2013-11-08 17:30 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=527D02FD.2000109@redhat.com \
--to=lersek@redhat.com \
--cc=afaerber@suse.de \
--cc=anthony@codemonkey.ws \
--cc=edk2-devel@lists.sourceforge.net \
--cc=jan.kiszka@siemens.com \
--cc=jljusten@gmail.com \
--cc=marcel.a@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.