qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Michael Clark <mjc@sifive.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Peter Crosthwaite <crosthwaite.peter@gmail.com>,
	Alexander Graf <agraf@suse.de>,
	Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function
Date: Wed, 9 May 2018 15:32:07 +1000	[thread overview]
Message-ID: <20180509053207.GA19425@umbus> (raw)
In-Reply-To: <CAFEAcA-bYHDSFL07f3BE_Lj-Dou+GeZod6cdX4-LSxzpRmQaDA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]

On Sun, May 06, 2018 at 04:04:02PM +0100, Peter Maydell wrote:
> On 6 May 2018 at 14:39, David Gibson <david@gibson.dropbear.id.au> wrote:
> > Well, I'm biased of course, but I think we'd be better off just
> > ditching the wrapper.  In its present form it is so limited as to not
> > really add any value.  If it was rewritten to do something useful
> > (e.g. handling reallocations), I think it would be even better if
> > done as an extension to libfdt itself so it can benefit everyone, not
> > just qemu.
> 
> Well, some of it is working around infelicities in libfdt's
> API (like all the getprop/setprop functions taking an offset
> value rather than a node name), but yes, it would be better
> to fix the libfdt API if possible.
> 
> > Although, that said, I'll re-iterate that I think qemu's fdt
> > manipulation is now sufficiently complex that it would be better off
> > using a "live" (dynamically allocated & pointer based) tree
> > representation that we just flatten immediately before loading it into
> > the guest.
> 
> This sounds to me like something that should be handled
> by libfdt.

No, it's really not.  libfdt is specifically for reading and writing
flattened trees in place, in flattened form.  That's the whole basis
of the design and it's directly responsible for some of the
infelicities you mention.

I was never intended as a general runtime t manipulation library.  A
libdt that does that (using an allocator and internal pointer
representation) would be a nice thing to have, but libfdt isn't it.
Obviously being able to use libfdt to flatten and unflatten trees
would be a very good feature for such a library.

> internal representation of the data structure is, I just
> want to be able to (a) hand it an fdt read in from a file
> (b) call various functions to modify the data structure
> and then (c) write the resulting thing out to an fdt in
> guest memory. Whether libfdt prefers to do that by
> modifying the flat representation or by converting into
> a dynamically allocated unflattened tree and back again
> is something I'd rather leave to it as an implementation
> detail.

Sort of.  But there design constraints in libfdt which means that's
not really feasible.  Such as:
  * not requiring an allocator
  * not requiring a "read / unflatten" pass before reading values from
    an fdt
  * not requiring allocation or creation of a "context" state for
    manipulating a tree

Those aren't aren't really relevant to qemu and perhaps aren't part of
what you'd usually think of as the API.  But they do matter to real
libfdt use cases.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-05-09  5:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-04  1:19 [Qemu-devel] [PATCH] device_tree: Add qemu_fdt_totalsize function Michael Clark
2018-05-05 10:44 ` Peter Maydell
2018-05-05 11:48   ` David Gibson
2018-05-05 21:59     ` Michael Clark
2018-05-05 23:03       ` Michael Clark
2018-05-06 12:23       ` Peter Maydell
2018-05-06 13:39         ` David Gibson
2018-05-06 15:04           ` Peter Maydell
2018-05-09  5:32             ` David Gibson [this message]
2018-05-09 11:23               ` Peter Maydell
2018-06-18  2:58                 ` David Gibson
2018-05-06 13:53       ` David Gibson

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=20180509053207.GA19425@umbus \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=alistair.francis@wdc.com \
    --cc=crosthwaite.peter@gmail.com \
    --cc=mjc@sifive.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).