From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34953) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv7Bu-00085E-6h for qemu-devel@nongnu.org; Wed, 08 Feb 2012 08:04:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rv7Bq-0001Y0-4y for qemu-devel@nongnu.org; Wed, 08 Feb 2012 08:04:06 -0500 Received: from mail-bk0-f45.google.com ([209.85.214.45]:51731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv7Bp-0001Xv-NJ for qemu-devel@nongnu.org; Wed, 08 Feb 2012 08:04:01 -0500 Received: by bkue19 with SMTP id e19so321932bku.4 for ; Wed, 08 Feb 2012 05:04:00 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1116A54F-BE1E-4620-BDC8-6B6A1A63D3B6@suse.de> References: <1328687721-16030-1-git-send-email-peter.crosthwaite@petalogix.com> <201202081139.49413.paul@codesourcery.com> <201202081228.00120.paul@codesourcery.com> <1116A54F-BE1E-4620-BDC8-6B6A1A63D3B6@suse.de> Date: Wed, 8 Feb 2012 23:04:00 +1000 Message-ID: From: Peter Crosthwaite Content-Type: multipart/alternative; boundary=0015175cd5ce7c01ed04b87387d3 Subject: Re: [Qemu-devel] [RFC PATCH] arm boot: added QOM device definition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, Paul Brook , qemu-devel@nongnu.org --0015175cd5ce7c01ed04b87387d3 Content-Type: text/plain; charset=ISO-8859-1 On Wed, Feb 8, 2012 at 10:41 PM, Alexander Graf wrote: > > On 08.02.2012, at 13:27, Paul Brook wrote: > > >> 2012/2/8 Paul Brook > >> > >>>>> I suspect we want to replace the arm_load_kernel call with an > >>>>> arm_linux_loader device with appropriate properties. > >>>> > >>>> Ok, so does this mean the machine model would still explicitly > >>>> instantiate the bootloader device? > >>> > >>> Yes. Bootloaders inherently have machine specific knowledge. They > need > >>> to know ram location, board ID, secondary CPU boot protocols, etc. > >>> Requiring the user specify all these things separately from the rest of > >>> the machine description is IMO not acceptable. > >> > >> So what im suggesting here is that machines export these properties to a > >> globally accessible location. Perhaps via the machine opts mechanism? > Then > >> we are in a best of both worls situation where machine models do not > need > >> bootloader awareness yet bootloaders can still query qemu for ram_size, > >> smp#, board_id and friends. > > > > Hmm, I suppose this might work. I'm not sure what you think the benefit > of > > this is though. Fact is the machine needs to have bootloader awareness, > > whether it be instantating an object or setting magic variables. > > Having devices rummage around in global state feels messy. I'd much > rather > > use actual properties on the device. IMO changing the bootloader is > similar > > complexity to (say) changing a UART. i.e. it's a board-level change not > an > > end-user level change. Board-level changes are something that will > happen > > after QOM conversion, i.e. when we replace machine->init with a board > config > > file. > > > Yeah, basically the variable flow goes: > > vl.c -> machine_opts -> machine_init() -> device properties -> > device_init() > So that the machine init function that creates the bootloader device > enumerates the machine_opts (just like is done in Peter's patches) and then > passes those on to the bootloader device as device properties. > > So in patch 4/4 in Peters series where he adds a new bootloader feature (the -dtb switch) its done slightly differently, the machine model does not handle the machine_opts at all, i.e. The machine model has no awareness of this dtb argument. Instead the arm boot loader directly queries the machine_opts API itself: @@ -251,6 +317,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info) exit(1); } + info->dtb_filename = qemu_opt_get(qemu_opts_find( qemu_find_opts("machine"), + 0), "dtb"); + There is no path through the machine_init for this particular property. What I am suggesting is that a similar approach is take for machine model set properties (such as ram_size), but instead of the command line setting the props its done by the machine model. The machine model qemu_opt_set() the ram_size = whatever. Then the bootloader qemu_opt_get()s it. If you did this for the key properties related to boot then you would remove the need for machines to have awareness of their boot process. > The rationale behind machine opts is that they're basically a dynamic > number of properties for the not-yet-existing machine object. > > > Alex > > --0015175cd5ce7c01ed04b87387d3 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Wed, Feb 8, 2012 at 10:41 PM, Alexand= er Graf <agraf@suse.de> wrote:

On 08.02.2012, at 13:27, Paul Brook wrote:

>> 2012/2/8 Paul Brook <paul@codesourcery.com>
>>
>>>>> I suspect we want to replace the arm_load_kernel call = with an
>>>>> arm_linux_loader device with appropriate properties. >>>>
>>>> Ok, so does this mean the machine model would still explic= itly
>>>> instantiate the bootloader device?
>>>
>>> Yes. =A0Bootloaders inherently have machine specific knowledge= . =A0They need
>>> to know ram location, board ID, secondary CPU boot protocols, = etc.
>>> Requiring the user specify all these things separately from th= e rest of
>>> the machine description is IMO not acceptable.
>>
>> So what im suggesting here is that machines export these propertie= s to a
>> globally accessible location. Perhaps via the machine opts mechani= sm? Then
>> we are in a best of both worls situation where machine models do n= ot need
>> bootloader awareness yet bootloaders can still query qemu for ram_= size,
>> smp#, board_id and friends.
>
> Hmm, I suppose this might work. =A0I'm not sure what you think the= benefit of
> this is though. =A0Fact is the machine needs to have bootloader awaren= ess,
> whether it be instantating an object or setting magic variables.
> Having devices rummage around in global state feels messy. =A0I'd = much rather
> use actual properties on the device. =A0IMO changing the bootloader is= similar
> complexity to (say) changing a UART. i.e. it's a board-level chang= e not an
> end-user level change. =A0Board-level changes are something that will = happen
> after QOM conversion, i.e. when we replace machine->init with a boa= rd config
> file.


Yeah, basically the variable flow goes:

=A0vl.c -> machine_opts -> machine_init() -> device properties -&= gt; device_init()
=A0
So that the machine init function that creates the bootloader device enumer= ates the machine_opts (just like is done in Peter's patches) and then p= asses those on to the bootloader device as device properties.


So in patch 4/4 in Peters series where= he adds a new bootloader feature (the -dtb switch) its done slightly diffe= rently, the machine model does not handle the machine_opts at all, i.e. The= machine model has no awareness of this dtb argument. Instead the arm boot = loader directly queries the machine_opts API itself:

@@ -251,6 +317,9 @@ void arm_load_kernel(CP= UState *env, struct arm_boot_info *info)
=A0 = =A0 =A0 =A0 exit(1);
=A0 =A0 }
=
+ =A0 =A0info->dtb_filename =3D qemu_opt_get(qemu_opts_find(= qemu_find_opts("machine"),
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0), "dtb");
+

There is no path through the machine_init for thi= s particular property. What I am suggesting is that a similar approach is t= ake for machine model set properties (such as ram_size), but instead of the= command line setting the props its done by the machine model. The machine = model qemu_opt_set() the ram_size =3D whatever. Then the bootloader qemu_op= t_get()s it. If you did this for the key properties related to boot then yo= u would remove the need for machines to have awareness of their boot proces= s.
=A0
The rationale behind machine opts is that they're basically a dynamic n= umber of properties for the not-yet-existing machine object.


Alex


--0015175cd5ce7c01ed04b87387d3--