qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm
Date: Thu, 21 Jun 2012 08:06:50 -0500	[thread overview]
Message-ID: <4FE31C6A.20207@codemonkey.ws> (raw)
In-Reply-To: <CAHFMJ7uZM0xsxbgCSCMNNotPZeMV0j-m11ah=Ya+4EP4seCSWQ@mail.gmail.com>

On 06/21/2012 07:31 AM, Paolo Bonzini wrote:
> (Sorry for breaking the thread).
>
>> This avoids the problem associated with having multiple target specific files
>> in a single directory with the current build system.
>
> What is exactly the problem?

Peter's got an ARM specific KVM device he wants to stick in hw/kvm.

Right now, kvm/ is all x86 specific and wants CONFIG_KVM && CONFIG_I386.  We 
gets this by doing:

hw/${TARGET}/Makefile.objs  <=  CONFIG_I386

And then within Makefile.objs:

obj-$(CONFIG_KVM) += kvm/

But this applies for the whole directory.  Previously, you did:

obj-$(CONFIG_KVM) += kvm/apic.o kvm/clock.o ...

The way you did it made it possible for hw/arm/Makefile.obj to have a different 
set of objects but also didn't use sub directory makefiles.

So this patch allows us to achieve CONFIG_KVM && CONFIG_I386 by doing:

hw/Makefiles.objs:

obj-$(CONFIG_KVM) += kvm/

hw/kvm/Makefiles.obj:

obj-$(CONFIG_I386) += apic.o clock.o

Which I think is actually more straight forward.  This gives us the logic we 
need and let's use us subdirectory makefiles too.

>
> I saw something about dependencies, I think that should be solved with
> something like
>
> $(foreach var, $(nested-vars), $(eval -include $(patsubst %.o, %.d, $($(var)))))
>
> at the very end of unnest-vars.

Already added that BTW :-)  That's a different thread.

>> We can eventually get rid of the hw/$BASE_ARCH/Makefiles.obj files too
>
> The goal should be to limit hw/$BASE_ARCH/Makefile.objs to hardware
> that is CPU-dependent and to board descriptions.
>
> I _think_ (but I don't have a checkout at hand) that hardware like
> virtio can use obj-$(CONFIG_VIRTIO) while staying in hw/Makefile.objs,
> but it should really be the only case of target-dependent file in hw/.
>   Everything else in hw/$BASE_ARCH should move to target-$BASE_ARCH/hw.
>   The steps should be as follows:

Yup, I'm trying to refactor some of this.

Most of what's in hw/$BASE_ARCH today is really just hardware that doesn't apply 
to any other targets but is not truly target specific.

We could introduce per-device CONFIG variables and update all of the 
default-configs/ but that's a big pain for marginal benefit.  Instead, I think 
what we want in the long term is to have machine-specific CONFIG variables. 
Something like CONFIG_PC or CONFIG_VERSITALE.

But in the very short term, CONFIG_I386 makes a good stepping stone to CONFIG_PC 
and let's use refactor the Makefiles such that we can introduce more granular 
CONFIG_* down the road without changing object locations.

I think that's really the way to think of it.  We start with big guards 
(CONFIG_I386) and over time break them down into smaller guards (CONFIG_PC).

> 1) Identify more groups of hardware that can be moved from
> hw/$BASE_ARCH to libhw. Move them.
>
> 2) At this point, hw/$BASE_ARCH/Makefile.objs should only refer to a)
> boards b) hardware that is CPU dependent c) KVM device models with
> host dependencies. Move the sources to hw/$BASE_ARCH, possibly
> hw/$BASE_ARCH/kvm, and remove the addprefix invocations from
> hw/$BASE_ARCH/Makefile.objs.
>
> 3) Move hw/$BASE_ARCH to target-$BASE_ARCH/hw.
>
> I think CONFIG_$BASE_ARCH is a bad idea because it violates the
> modularity that Juan introduced together with the config-devices.mak
> files.

On the contrary, I think it's the easiest way to improve our modularity.  See above.

Regards,

Anthony Liguori

>
> Paolo

  reply	other threads:[~2012-06-21 13:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-21 12:31 [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm Paolo Bonzini
2012-06-21 13:06 ` Anthony Liguori [this message]
2012-06-23  7:53 ` Paolo Bonzini
2012-06-23 10:30   ` Peter Maydell
2012-07-01 13:37     ` Paolo Bonzini
2012-07-01 13:49       ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2012-06-20 14:44 Anthony Liguori
2012-06-20 14:51 ` Andreas Färber
2012-06-20 15:01   ` Peter Maydell
2012-06-20 15:04     ` Andreas Färber
2012-06-20 15:07       ` Peter Maydell
2012-06-20 15:23         ` Andreas Färber
2012-06-20 15:20   ` Anthony Liguori
2012-06-20 15:09 ` Peter Maydell
     [not found] ` <CAFEAcA-MgQuEfca7bPtUrN-wwN0KVCvXWpcs8Y4tdWL+CbcGFw@mail.gmail.com>
     [not found]   ` <4FEDA01B.8010502@suse.de>
2012-07-01 14:10     ` Paolo Bonzini
     [not found]   ` <4FEDB5F0.1070407@codemonkey.ws>
2012-07-23 14:21     ` Peter Maydell
2012-07-31 17:28       ` Peter Maydell

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=4FE31C6A.20207@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=pbonzini@redhat.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).