From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57858) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv893-0004My-PI for qemu-devel@nongnu.org; Wed, 08 Feb 2012 09:05:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rv88y-0007Nm-65 for qemu-devel@nongnu.org; Wed, 08 Feb 2012 09:05:13 -0500 Received: from mail-bk0-f45.google.com ([209.85.214.45]:43609) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rv88x-0007NC-Rs for qemu-devel@nongnu.org; Wed, 08 Feb 2012 09:05:08 -0500 Received: by bkue19 with SMTP id e19so399750bku.4 for ; Wed, 08 Feb 2012 06:05:06 -0800 (PST) MIME-Version: 1.0 In-Reply-To: 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> <9643AC95-43AE-4FF5-A72F-8BB3A63596EC@suse.de> Date: Thu, 9 Feb 2012 00:05:06 +1000 Message-ID: From: Peter Crosthwaite Content-Type: multipart/alternative; boundary=000e0cdfc786f780e404b87461b2 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 --000e0cdfc786f780e404b87461b2 Content-Type: text/plain; charset=ISO-8859-1 On Wed, Feb 8, 2012 at 11:35 PM, Alexander Graf wrote: > > On 08.02.2012, at 14:30, Peter Crosthwaite wrote: > > > > On Wed, Feb 8, 2012 at 11:10 PM, Alexander Graf wrote: > >> >> On 08.02.2012, at 14:04, Peter Crosthwaite wrote: >> >> >> >> 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. >> >> >> Ah, I see. So he derived from the original proposal, oh well :). >> >> > Isn't this the best approach tho? If you changed it over to the other > flow, then you would end up with a change pattern where every machine model > would need to get and pass the new argument. This way the diff is limited > to the command line infrastructure and the bootloader. > > > If you want the smallest diff, just make things globals and call it a day. > This whole thing is a discussion around device architecture, right? > > So if we consider this bootloader a device and its -dtb argument a property of that device, then what you are implying is that every device property of every device in a machine must be managed by the machine model? Isn't the dynamic machine model work that is in progress is trying to get away the approach where fixed machine models have to micromanage all their attached devices? with the ultimate goal of -no-machine how will the bootloader get this dtb argument? > > >> 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. >> >> >> But that's exactly what we want. We want the machine to be aware of its >> boot process, because that's the one place that needs to instantiate the >> boot device, no? >> >> > More a case of the reverse isnt it? The bootloader needs to know about the > machine its generating code for, but the machine generation process doesnt > need to know about the software its going to run. The machine being aware > of the bootloader implementation and instantiating it you are putting in > place a policy where you forcing a particular bootflow on every user of > that machine. The hardware is placing policy on what software its running. > > In the case of arm boot you end up with a situation where you are trying > to write a bootloader that can handle every possible scenario, what I am > suggesting is arm_boot.c as it is becomes a linux specific bootloader and > other bootflows such as booting from elfs or raw images (or other unforseen > bootflows) are different bootloader devices. The choice of which bootloader > you use is driven by which -device you put down on command line. > > > Hrm. I wouldn't want to have to select different bootloader types using > -device. Today, we have autodetect code in almost all targets that checks > the binary and figures out what to load from it. So on x86 you just do > -kernel foo and if foo is a Linux kernel, it will run it using that loader, > while if it's a multiboot image, it will load it through that. > > Any reason you can't just upstream your specific bootloader code? > Its currently just a set of hacks to arm_boot.c, but for upstream acceptance you would need to add serveral more options to arm_boot (like -dtb) to be able to do this needed parameteristation. It would be tedious to have to editevery arm platform to pass in a suite of options from the machine model. > We can then worry about replacing the loader device at a different point > in time and model things easily now. > > > Alex > > --000e0cdfc786f780e404b87461b2 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

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

On 08.02.2012, = at 14:30, Peter Crosthwaite wrote:


<= br>
On Wed, Feb 8, 2012 at 11:10 PM, Alexander Gr= af <agraf@suse.de> wrote:

On 08.02.2012, = at 14:04, Peter Crosthwaite wrote:


<= br>
On Wed, Feb 8, 2012 at 10:41 PM, Alexander Gr= af <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(CPUState= *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.

=
Ah, I see. So he derived from the original proposal, oh well :).


Isn't this = the best approach tho? If you changed it over to the other flow, then you w= ould end up with a change pattern where every machine model would need to g= et and pass the new argument. This way the diff is limited to the command l= ine infrastructure and the bootloader.

If you want the smallest= diff, just make things globals and call it a day. This whole thing is a di= scussion around device architecture, right?


So if we consider this bootloader a = device and its -dtb argument a property of that device, then what you are i= mplying is that every device property of every device in a machine must be = managed by the machine model? Isn't the dynamic machine model work that= is in progress is trying to get away the approach where fixed machine mode= ls have to micromanage all their attached devices? with the ultimate goal o= f -no-machine how will the bootloader get this dtb argument?
=A0
What I am suggesting is that a similar approach is take for machine model s= et properties (such as ram_size), but instead of the command line setting t= he props its done by the machine model. The machine model qemu_opt_set() th= e ram_size =3D 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.

But that's exactly what we= want. We want the machine to be aware of its boot process, because that= 9;s the one place that needs to instantiate the boot device, no?


More a case of the r= everse isnt it? The bootloader needs to know about the machine its generati= ng code for, but the machine generation process doesnt need to know about t= he software its going to run. The machine being aware of the bootloader imp= lementation and instantiating it you are putting in place a policy where yo= u forcing a particular bootflow on every user of that machine. The hardware= is placing policy on what software its running.

In the case of arm boot you end up with a situation whe= re you are trying to write a bootloader that can handle every possible scen= ario, what I am suggesting is arm_boot.c as it is becomes a linux specific = bootloader and other bootflows such as booting from elfs or raw images (or = other unforseen bootflows) are different bootloader devices. The choice of = which bootloader you use is driven by which -device you put down on command= line.

Hrm. I wouldn't want to ha= ve to select different bootloader types using -device. Today, we have autod= etect code in almost all targets that checks the binary and figures out wha= t to load from it. So on x86 you just do -kernel foo and if foo is a Linux = kernel, it will run it using that loader, while if it's a multiboot ima= ge, it will load it through that.

Any reason you can't just upstream your specific bo= otloader code?

Its curren= tly just a set of hacks to arm_boot.c, but for upstream acceptance you woul= d need to add serveral more options to arm_boot (like -dtb) to be able to d= o this needed parameteristation. It would be tedious to have to editevery a= rm platform to pass in a suite of options from the machine model.
=A0
We can then worry about replacing the loader device at a d= ifferent point in time and model things easily now.


Alex


--000e0cdfc786f780e404b87461b2--