[Qemu-devel] How to split vl.h

[Qemu-devel] How to split vl.h

[Blue Swirl]

Hi,

With the automatic dependency rule installed, modifying vl.h causes
all files to be recompiled. This is of course the correct action, but
it's a major slowdown for development too.

How should we split vl.h into smaller pieces? Give each device a
header file, like m48t59? What about other stuff exported from vl.c?

Re: [Qemu-devel] How to split vl.h

[Jocelyn Mayer]

On Wed, 2007-10-31 at 20:12 +0200, Blue Swirl wrote:
> Hi,

Hi,

> With the automatic dependency rule installed, modifying vl.h causes
> all files to be recompiled. This is of course the correct action, but
> it's a major slowdown for development too.
> 
> How should we split vl.h into smaller pieces? Give each device a
> header file, like m48t59? What about other stuff exported from vl.c?

It seems to me that a lot of things could go back in headers in the hw
subdirectory. A quick and simple approach could be to create a hw/hw.h
file and move everything from vl.h that is only used in the hw
subdirectory. This way, we would break the strong compile-time
dependencies between the hardware library and the rest of the emulation
code.

I think there are some things, like PC serial, USB, ... that could be
left in this kind of header without any issue: they're widely used and
are not likely to change too often.
Some other architecture specific devices would better have their own
header (like the m48t59.h file) or maybe a target dependant header could
be sufficient (quite like the ppc_mac.h I added a few days ago). Having
this kind of header could also be great so we could put the machines
extern definition in it and only include this header file in vl.c (not
vl.h !) to get, for example, the defined machine list for the current
target architecture (and not all possible machines as it is now)...

It also seems that some hardware dependent stuffs are currently
implemented in vl.c, like the bloc devices or USB registration code,
that prevent the related definitions to be isolated in the hw
subdirectory. Splitting vl.c to move those functions into separate
file(s) in hw would allow moving a lot of definitions in the hw
subdirectory. Ideally, vl.c should be hw / target independant and call
the proper routines to do all hardware implementation dependant
initialisations, imho.

Those are just ideas, maybe some are too complicated to be done in a
short timeline; and there may be better solutions... if any other
ideas...

-- 
Jocelyn Mayer <l_indien@magic.fr>

Re: [Qemu-devel] How to split vl.h

[Thiemo Seufer]

Blue Swirl wrote:
> Hi,
> 
> With the automatic dependency rule installed, modifying vl.h causes
> all files to be recompiled. This is of course the correct action, but
> it's a major slowdown for development too.
> 
> How should we split vl.h into smaller pieces? Give each device a
> header file, like m48t59?

Functions which are only called from below /hw should go in /hw/<device>.h,
other device stuff should probably go in a separate toplevel header,
maybe hw.h. That way we get something which could evolve in the
interface of a hw-library in future.

> What about other stuff exported from vl.c?

I think the best answer is to cut vl.c in logical chunks before starting
to split vl.h further.


Thiemo

Re: [Qemu-devel] How to split vl.h

[Fabrice Bellard]

Blue Swirl wrote:
> Hi,
> 
> With the automatic dependency rule installed, modifying vl.h causes
> all files to be recompiled. This is of course the correct action, but
> it's a major slowdown for development too.

There must be an option in the Makefile to disable the automatic
dependency check.

> How should we split vl.h into smaller pieces? Give each device a
> header file, like m48t59? What about other stuff exported from vl.c?

The net result is that you will have dozens of header files with only
one line in them as most devices only export one function.

Regards,

Fabrice.

Re: [Qemu-devel] How to split vl.h

[Stefan Weil]

Fabrice Bellard schrieb:
> Blue Swirl wrote:
>> Hi,
>>
>> With the automatic dependency rule installed, modifying vl.h causes
>> all files to be recompiled. This is of course the correct action, but
>> it's a major slowdown for development too.
> There must be an option in the Makefile to disable the automatic
> dependency check.
>From my own experience, I can tell that the automatic
dependency check is not really a problem, but makes
things much easier and  safer (I used it for more than
a year now).

I never missed a Makefile option to disable it. Of course,
changes of vl.h are somehow annoying when they force
a rebuild of nearly everything. But in most cases I focus
on one target architecture (e.g. mipsel-softmmu), so
compilation takes not much time even when everything
is compiled. And you always can make a "touch *.o */*.o"
if you know what you do and want to prevent a new
compilation (or use a private modification of the Makefiles).

Options make things more complicated - I don't think we need one here.
>> How should we split vl.h into smaller pieces? Give each device a
>> header file, like m48t59? What about other stuff exported from vl.c?
> The net result is that you will have dozens of header files with only
> one line in them as most devices only export one function.
So you can group headers - one header for network emulations,
one for graphics, ...

We had this discussion about splitting vl.h before, and I
still think it would be good.
>
> Regards,
>
> Fabrice.
Regards
Stefan

Re: [Qemu-devel] How to split vl.h

[andrzej zaborowski]

Hi,

On 01/11/2007, Stefan Weil <weil@mail.berlios.de> wrote:
> Fabrice Bellard schrieb:
> > Blue Swirl wrote:
> >> Hi,
> >>
> >> With the automatic dependency rule installed, modifying vl.h causes
> >> all files to be recompiled. This is of course the correct action, but
> >> it's a major slowdown for development too.
> > There must be an option in the Makefile to disable the automatic
> > dependency check.
> From my own experience, I can tell that the automatic
> dependency check is not really a problem, but makes
> things much easier and  safer (I used it for more than
> a year now).

>From my experience I can tell that working with no automatic
dependency checks is also not so bad, it was just a bit of getting
used to it. Qemu is actually one of very few projects I've seen with
no automatic dependency checks in the makefile, but it stopped being
annoying after little time.

The huge vl.h with all declarations in one file is also unlike all
other projects but admittedly it's pretty comfortable (unless it
forces you to rebuild whole world on every modification). So I don't
mind if it stays the way it was until now, but I also welcome an
overhaul.

>
> I never missed a Makefile option to disable it. Of course,
> changes of vl.h are somehow annoying when they force
> a rebuild of nearly everything. But in most cases I focus
> on one target architecture (e.g. mipsel-softmmu), so
> compilation takes not much time even when everything
> is compiled. And you always can make a "touch *.o */*.o"
> if you know what you do and want to prevent a new
> compilation (or use a private modification of the Makefiles).
>
> Options make things more complicated - I don't think we need one here.

Adding this particular option wouldn't be intrusive.

> >> How should we split vl.h into smaller pieces? Give each device a
> >> header file, like m48t59? What about other stuff exported from vl.c?
> > The net result is that you will have dozens of header files with only
> > one line in them as most devices only export one function.
> So you can group headers - one header for network emulations,
> one for graphics, ...

The logical division is not so practical here, it will again mean
rebuilding everything that uses graphics if one adapter changes.

Maybe a more clever dependency tracking is possible similar to that in
Linux or Elinks.

>
> We had this discussion about splitting vl.h before, and I
> still think it would be good.
> >
> > Regards,
> >
> > Fabrice.
> Regards
> Stefan
>
>
>
>

Regards

Re: [Qemu-devel] How to split vl.h

[Blue Swirl]

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

On 11/1/07, Fabrice Bellard <fabrice@bellard.org> wrote:
> Blue Swirl wrote:
> > Hi,
> >
> > With the automatic dependency rule installed, modifying vl.h causes
> > all files to be recompiled. This is of course the correct action, but
> > it's a major slowdown for development too.
>
> There must be an option in the Makefile to disable the automatic
> dependency check.
>
> > How should we split vl.h into smaller pieces? Give each device a
> > header file, like m48t59? What about other stuff exported from vl.c?
>
> The net result is that you will have dozens of header files with only
> one line in them as most devices only export one function.

I have another solution: include all architecture specific files from
the main file. This actually makes the compilation faster and the
resulting binary is smaller (maybe faster). Changing the architecture
specific code needs no changes to vl.h, just a recompile of sun4m.c,
but this is instantaneous on my machine. Automatic dependencies also
handle this case. I guess some may find this style pretty ugly.

Similar approach could be taken with the network adapters, sound
cards, Slirp (for the speed, not vl.h effect) etc. by introducing .c
files that include all the others.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sun4m_include_code.diff --]
[-- Type: text/x-diff; name=sun4m_include_code.diff, Size: 12839 bytes --]

Index: qemu/Makefile.target
===================================================================
--- qemu.orig/Makefile.target	2007-11-04 08:17:00.000000000 +0000
+++ qemu/Makefile.target	2007-11-04 08:20:41.000000000 +0000
@@ -505,9 +505,7 @@
 VL_OBJS+= fdc.o mc146818rtc.o serial.o m48t59.o
 VL_OBJS+= cirrus_vga.o parallel.o ptimer.o
 else
-VL_OBJS+= sun4m.o tcx.o pcnet.o iommu.o m48t59.o slavio_intctl.o
-VL_OBJS+= slavio_timer.o slavio_serial.o slavio_misc.o fdc.o esp.o sparc32_dma.o
-VL_OBJS+= cs4231.o ptimer.o
+VL_OBJS+= sun4m.o
 endif
 endif
 ifeq ($(TARGET_BASE_ARCH), arm)
Index: qemu/hw/cs4231.c
===================================================================
--- qemu.orig/hw/cs4231.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/cs4231.c	2007-11-04 08:20:41.000000000 +0000
@@ -164,7 +164,7 @@
     return 0;
 }
 
-void cs_init(target_phys_addr_t base, int irq, void *intctl)
+static void cs_init(target_phys_addr_t base, int irq, void *intctl)
 {
     int cs_io_memory;
     CSState *s;
Index: qemu/hw/esp.c
===================================================================
--- qemu.orig/hw/esp.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/esp.c	2007-11-04 08:20:41.000000000 +0000
@@ -551,7 +551,7 @@
     return 0;
 }
 
-void esp_scsi_attach(void *opaque, BlockDriverState *bd, int id)
+static void esp_scsi_attach(void *opaque, BlockDriverState *bd, int id)
 {
     ESPState *s = (ESPState *)opaque;
 
@@ -574,8 +574,8 @@
     s->scsi_dev[id] = scsi_disk_init(bd, 0, esp_command_complete, s);
 }
 
-void *esp_init(BlockDriverState **bd, target_phys_addr_t espaddr,
-               void *dma_opaque, qemu_irq irq, qemu_irq *reset)
+static void *esp_init(BlockDriverState **bd, target_phys_addr_t espaddr,
+                      void *dma_opaque, qemu_irq irq, qemu_irq *reset)
 {
     ESPState *s;
     int esp_io_memory;
Index: qemu/hw/iommu.c
===================================================================
--- qemu.orig/hw/iommu.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/iommu.c	2007-11-04 08:20:41.000000000 +0000
@@ -244,8 +244,8 @@
     s->regs[IOMMU_AFAR] = addr;
 }
 
-void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
-                           uint8_t *buf, int len, int is_write)
+static void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
+                                  uint8_t *buf, int len, int is_write)
 {
     int l;
     uint32_t flags;
@@ -311,7 +311,7 @@
     s->regs[IOMMU_CTRL] = IOMMU_VERSION;
 }
 
-void *iommu_init(target_phys_addr_t addr)
+static void *iommu_init(target_phys_addr_t addr)
 {
     IOMMUState *s;
     int iommu_io_memory;
Index: qemu/hw/slavio_intctl.c
===================================================================
--- qemu.orig/hw/slavio_intctl.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/slavio_intctl.c	2007-11-04 08:20:41.000000000 +0000
@@ -201,7 +201,7 @@
     slavio_intctlm_mem_writel,
 };
 
-void slavio_pic_info(void *opaque)
+static void slavio_pic_info(void *opaque)
 {
     SLAVIO_INTCTLState *s = opaque;
     int i;
@@ -212,7 +212,7 @@
     term_printf("master: pending 0x%08x, disabled 0x%08x\n", s->intregm_pending, s->intregm_disabled);
 }
 
-void slavio_irq_info(void *opaque)
+static void slavio_irq_info(void *opaque)
 {
 #ifndef DEBUG_IRQ_COUNT
     term_printf("irq statistic code not compiled.\n");
@@ -349,10 +349,11 @@
     slavio_check_interrupts(s);
 }
 
-void *slavio_intctl_init(target_phys_addr_t addr, target_phys_addr_t addrg,
-                         const uint32_t *intbit_to_level,
-                         qemu_irq **irq, qemu_irq **cpu_irq,
-                         qemu_irq **parent_irq, unsigned int cputimer)
+static void *slavio_intctl_init(target_phys_addr_t addr,
+                                target_phys_addr_t addrg,
+                                const uint32_t *intbit_to_level,
+                                qemu_irq **irq, qemu_irq **cpu_irq,
+                                qemu_irq **parent_irq, unsigned int cputimer)
 {
     int slavio_intctl_io_memory, slavio_intctlm_io_memory, i;
     SLAVIO_INTCTLState *s;
Index: qemu/hw/slavio_misc.c
===================================================================
--- qemu.orig/hw/slavio_misc.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/slavio_misc.c	2007-11-04 08:20:41.000000000 +0000
@@ -70,7 +70,7 @@
     s->config = s->aux1 = s->aux2 = s->mctrl = 0;
 }
 
-void slavio_set_power_fail(void *opaque, int power_failing)
+static void slavio_set_power_fail(void *opaque, int power_failing)
 {
     MiscState *s = opaque;
 
@@ -212,8 +212,9 @@
     return 0;
 }
 
-void *slavio_misc_init(target_phys_addr_t base, target_phys_addr_t power_base,
-                       qemu_irq irq)
+static void *slavio_misc_init(target_phys_addr_t base,
+                              target_phys_addr_t power_base,
+                              qemu_irq irq)
 {
     int slavio_misc_io_memory;
     MiscState *s;
Index: qemu/hw/slavio_serial.c
===================================================================
--- qemu.orig/hw/slavio_serial.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/slavio_serial.c	2007-11-04 08:20:41.000000000 +0000
@@ -590,8 +590,9 @@
 
 }
 
-SerialState *slavio_serial_init(target_phys_addr_t base, qemu_irq irq,
-                                CharDriverState *chr1, CharDriverState *chr2)
+static SerialState *slavio_serial_init(target_phys_addr_t base, qemu_irq irq,
+                                       CharDriverState *chr1,
+                                       CharDriverState *chr2)
 {
     int slavio_serial_io_memory, i;
     SerialState *s;
@@ -759,7 +760,7 @@
     put_queue(s, 0);
 }
 
-void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq)
+static void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq)
 {
     int slavio_serial_io_memory, i;
     SerialState *s;
Index: qemu/hw/slavio_timer.c
===================================================================
--- qemu.orig/hw/slavio_timer.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/slavio_timer.c	2007-11-04 08:20:41.000000000 +0000
@@ -325,8 +325,8 @@
     return s;
 }
 
-void slavio_timer_init_all(target_phys_addr_t base, qemu_irq master_irq,
-                           qemu_irq *cpu_irqs)
+static void slavio_timer_init_all(target_phys_addr_t base, qemu_irq master_irq,
+                                  qemu_irq *cpu_irqs)
 {
     SLAVIO_TIMERState *master;
     unsigned int i;
Index: qemu/hw/sparc32_dma.c
===================================================================
--- qemu.orig/hw/sparc32_dma.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/sparc32_dma.c	2007-11-04 08:20:41.000000000 +0000
@@ -128,7 +128,7 @@
     }
 }
 
-void espdma_memory_read(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 {
     DMAState *s = opaque;
 
@@ -139,7 +139,7 @@
     s->dmaregs[1] += len;
 }
 
-void espdma_memory_write(void *opaque, uint8_t *buf, int len)
+static void espdma_memory_write(void *opaque, uint8_t *buf, int len)
 {
     DMAState *s = opaque;
 
@@ -237,7 +237,7 @@
     return 0;
 }
 
-void *sparc32_dma_init(target_phys_addr_t daddr, qemu_irq parent_irq,
+static void *sparc32_dma_init(target_phys_addr_t daddr, qemu_irq parent_irq,
                        void *iommu, qemu_irq **dev_irq, qemu_irq **reset)
 {
     DMAState *s;
Index: qemu/hw/sun4m.c
===================================================================
--- qemu.orig/hw/sun4m.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/sun4m.c	2007-11-04 08:20:41.000000000 +0000
@@ -22,6 +22,40 @@
  * THE SOFTWARE.
  */
 #include "vl.h"
+/* iommu.c */
+static void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
+                                  uint8_t *buf, int len, int is_write);
+static inline void sparc_iommu_memory_read(void *opaque,
+                                           target_phys_addr_t addr,
+                                           uint8_t *buf, int len)
+{
+    sparc_iommu_memory_rw(opaque, addr, buf, len, 0);
+}
+
+static inline void sparc_iommu_memory_write(void *opaque,
+                                            target_phys_addr_t addr,
+                                            uint8_t *buf, int len)
+{
+    sparc_iommu_memory_rw(opaque, addr, buf, len, 1);
+}
+
+/* sparc32_dma.c */
+static void espdma_memory_read(void *opaque, uint8_t *buf, int len);
+static void espdma_memory_write(void *opaque, uint8_t *buf, int len);
+
+#include "tcx.c"
+#include "iommu.c"
+#include "m48t59.c"
+#include "slavio_intctl.c"
+#include "slavio_timer.c"
+#include "slavio_serial.c"
+#include "slavio_misc.c"
+#include "fdc.c"
+#include "esp.c"
+#include "sparc32_dma.c"
+#include "cs4231.c"
+#include "ptimer.c"
+
 //#define DEBUG_IRQ
 
 /*
Index: qemu/hw/tcx.c
===================================================================
--- qemu.orig/hw/tcx.c	2007-11-04 08:17:00.000000000 +0000
+++ qemu/hw/tcx.c	2007-11-04 08:20:41.000000000 +0000
@@ -488,9 +488,10 @@
     tcx_dummy_writel,
 };
 
-void tcx_init(DisplayState *ds, target_phys_addr_t addr, uint8_t *vram_base,
-              unsigned long vram_offset, int vram_size, int width, int height,
-              int depth)
+static void tcx_init(DisplayState *ds, target_phys_addr_t addr,
+                     uint8_t *vram_base,
+                     unsigned long vram_offset, int vram_size,
+                     int width, int height, int depth)
 {
     TCXState *s;
     int io_memory, dummy_memory;
Index: qemu/vl.h
===================================================================
--- qemu.orig/vl.h	2007-11-04 08:17:00.000000000 +0000
+++ qemu/vl.h	2007-11-04 08:20:41.000000000 +0000
@@ -1245,37 +1245,6 @@
 /* sun4m.c */
 extern QEMUMachine ss5_machine, ss10_machine;
 
-/* iommu.c */
-void *iommu_init(target_phys_addr_t addr);
-void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
-                                 uint8_t *buf, int len, int is_write);
-static inline void sparc_iommu_memory_read(void *opaque,
-                                           target_phys_addr_t addr,
-                                           uint8_t *buf, int len)
-{
-    sparc_iommu_memory_rw(opaque, addr, buf, len, 0);
-}
-
-static inline void sparc_iommu_memory_write(void *opaque,
-                                            target_phys_addr_t addr,
-                                            uint8_t *buf, int len)
-{
-    sparc_iommu_memory_rw(opaque, addr, buf, len, 1);
-}
-
-/* tcx.c */
-void tcx_init(DisplayState *ds, target_phys_addr_t addr, uint8_t *vram_base,
-              unsigned long vram_offset, int vram_size, int width, int height,
-              int depth);
-
-/* slavio_intctl.c */
-void *slavio_intctl_init(target_phys_addr_t addr, target_phys_addr_t addrg,
-                         const uint32_t *intbit_to_level,
-                         qemu_irq **irq, qemu_irq **cpu_irq,
-                         qemu_irq **parent_irq, unsigned int cputimer);
-void slavio_pic_info(void *opaque);
-void slavio_irq_info(void *opaque);
-
 /* loader.c */
 int get_image_size(const char *filename);
 int load_image(const char *filename, uint8_t *addr);
@@ -1284,38 +1253,11 @@
 int load_aout(const char *filename, uint8_t *addr);
 int load_uboot(const char *filename, target_ulong *ep, int *is_linux);
 
-/* slavio_timer.c */
-void slavio_timer_init_all(target_phys_addr_t base, qemu_irq master_irq,
-                           qemu_irq *cpu_irqs);
-
-/* slavio_serial.c */
-SerialState *slavio_serial_init(target_phys_addr_t base, qemu_irq irq,
-                                CharDriverState *chr1, CharDriverState *chr2);
-void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq);
-
-/* slavio_misc.c */
-void *slavio_misc_init(target_phys_addr_t base, target_phys_addr_t power_base,
-                       qemu_irq irq);
-void slavio_set_power_fail(void *opaque, int power_failing);
-
-/* esp.c */
-void esp_scsi_attach(void *opaque, BlockDriverState *bd, int id);
-void *esp_init(BlockDriverState **bd, target_phys_addr_t espaddr,
-               void *dma_opaque, qemu_irq irq, qemu_irq *reset);
-
 /* sparc32_dma.c */
-void *sparc32_dma_init(target_phys_addr_t daddr, qemu_irq parent_irq,
-                       void *iommu, qemu_irq **dev_irq, qemu_irq **reset);
 void ledma_memory_read(void *opaque, target_phys_addr_t addr,
                        uint8_t *buf, int len, int do_bswap);
 void ledma_memory_write(void *opaque, target_phys_addr_t addr,
                         uint8_t *buf, int len, int do_bswap);
-void espdma_memory_read(void *opaque, uint8_t *buf, int len);
-void espdma_memory_write(void *opaque, uint8_t *buf, int len);
-
-/* cs4231.c */
-void cs_init(target_phys_addr_t base, int irq, void *intctl);
-
 /* sun4u.c */
 extern QEMUMachine sun4u_machine;
 

Re: [Qemu-devel] How to split vl.h

[Paul Brook]

> I have another solution: include all architecture specific files from
> the main file.

I'd really rather not do this. I doubt it's going to be a win, as now you have 
to recompile the whole thing every time you change the implementation. At 
least with vl.h you only have to recompile when you change the interface.

Paul

Re: [Qemu-devel] How to split vl.h

[J. Mayer]

On Sun, 2007-11-04 at 12:17 +0000, Paul Brook wrote:
> > I have another solution: include all architecture specific files from
> > the main file.
> 
> I'd really rather not do this. I doubt it's going to be a win, as now you have 
> to recompile the whole thing every time you change the implementation. At 
> least with vl.h you only have to recompile when you change the interface.

What I feel about this is that adding a hw/hw.h, included in all hw/*.c
files would greatly improve the situation: changing vl.h would lead to
recompile the core emulator object files, changing hw/hw.h would lead to
recompile the hardware library.
A first pass to do this could be achieved with a minimal effort, just
moving all prototypes and structure definitions that could be moved
without having to change vl.c. Then, things could be refined to move
some hardware specific stuffs from vl.c to hw subdirectory: for example,
the USB or display registration functions could go in a file in hw which
would avoid USBDevice or DisplayState to be defined globally.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] How to split vl.h

[Paul Brook]

On Sunday 04 November 2007, J. Mayer wrote:
> On Sun, 2007-11-04 at 12:17 +0000, Paul Brook wrote:
> > > I have another solution: include all architecture specific files from
> > > the main file.
> >
> > I'd really rather not do this. I doubt it's going to be a win, as now you
> > have to recompile the whole thing every time you change the
> > implementation. At least with vl.h you only have to recompile when you
> > change the interface.
>
> What I feel about this is that adding a hw/hw.h, included in all hw/*.c
> files would greatly improve the situation: changing vl.h would lead to
> recompile the core emulator object files, changing hw/hw.h would lead to
> recompile the hardware library.

Well, most of the "core emulator" doesn't depend on vl.h anyway. It's just the 
device emulation and host disk/display code.

I not sure a single hw/hw.h file will give any benefit because there's a fair 
amount of interfacing between the target devices emulation and the host side 
interaction code. i.e. there's not much that's only used inside hw/.
hw/ is about as big as most of the rest of qemu put together, so splitting 
that is probably going to get the biggest wins.

How about dividing things up by category? e.g. Have header files for all of 
(in no particular order):

- Things includes by everything
- The block IO layer.
- The character IO layer
- Network IO layer.
- Display interface.
- Generic Device infrastructure (memory mapping, IRQs, etc). Maybe subdivide 
for common busses like scsi/usb/pci/i2c.
- Prototypes for device emulation init routines.

Each file can then include whichever categories it needs.

Paul

Re: [Qemu-devel] How to split vl.h

[J. Mayer]

On Sun, 2007-11-04 at 17:54 +0000, Paul Brook wrote:
> On Sunday 04 November 2007, J. Mayer wrote:
> > On Sun, 2007-11-04 at 12:17 +0000, Paul Brook wrote:
> > > > I have another solution: include all architecture specific files from
> > > > the main file.
> > >
> > > I'd really rather not do this. I doubt it's going to be a win, as now you
> > > have to recompile the whole thing every time you change the
> > > implementation. At least with vl.h you only have to recompile when you
> > > change the interface.
> >
> > What I feel about this is that adding a hw/hw.h, included in all hw/*.c
> > files would greatly improve the situation: changing vl.h would lead to
> > recompile the core emulator object files, changing hw/hw.h would lead to
> > recompile the hardware library.
> 
> Well, most of the "core emulator" doesn't depend on vl.h anyway. It's just the 
> device emulation and host disk/display code.
> 
> I not sure a single hw/hw.h file will give any benefit because there's a fair 
> amount of interfacing between the target devices emulation and the host side 
> interaction code. i.e. there's not much that's only used inside hw/.
> hw/ is about as big as most of the rest of qemu put together, so splitting 
> that is probably going to get the biggest wins.

hw library contains a lot of code but is not all is compiled for all
targets.

> How about dividing things up by category? e.g. Have header files for all of 
> (in no particular order):
> 
> - Things includes by everything
> - The block IO layer.
> - The character IO layer
> - Network IO layer.
> - Display interface.
> - Generic Device infrastructure (memory mapping, IRQs, etc). Maybe subdivide 
> for common busses like scsi/usb/pci/i2c.
> - Prototypes for device emulation init routines.
> 
> Each file can then include whichever categories it needs.

Yes, it could be a great solution. Mine was just the "quick and less
effort" proposal !
It seems you also should have headers for target specific declarations.
This would avoid recompiling all targets when working on devices
specific to only one or a few of them.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] How to split vl.h

[Paul Brook]

> > I not sure a single hw/hw.h file will give any benefit because there's a
> > fair amount of interfacing between the target devices emulation and the
> > host side interaction code. i.e. there's not much that's only used inside
> > hw/. hw/ is about as big as most of the rest of qemu put together, so
> > splitting that is probably going to get the biggest wins.
>
> hw library contains a lot of code but is not all is compiled for all
> targets.

*shrug*. When I'm developing I generally only build the target I'm interested 
in anyway. If I'm doing a verification build before committing I'll build 
from scratch anyway. I've been bitten by faulty dependency checking 
(automatic or otherwise) often enough that I don't trust it for anything 
important.

Paul

Re: [Qemu-devel] How to split vl.h

[Thiemo Seufer]

Paul Brook wrote:
> > > I not sure a single hw/hw.h file will give any benefit because there's a
> > > fair amount of interfacing between the target devices emulation and the
> > > host side interaction code. i.e. there's not much that's only used inside
> > > hw/. hw/ is about as big as most of the rest of qemu put together, so
> > > splitting that is probably going to get the biggest wins.
> >
> > hw library contains a lot of code but is not all is compiled for all
> > targets.
> 
> *shrug*. When I'm developing I generally only build the target I'm interested 
> in anyway. If I'm doing a verification build before committing I'll build 
> from scratch anyway. I've been bitten by faulty dependency checking 
> (automatic or otherwise) often enough that I don't trust it for anything 
> important.

Dependency checking appears to work well now.


Thiemo

Re: [Qemu-devel] How to split vl.h

[Thiemo Seufer]

Blue Swirl wrote:
> On 11/1/07, Fabrice Bellard <fabrice@bellard.org> wrote:
> > Blue Swirl wrote:
> > > Hi,
> > >
> > > With the automatic dependency rule installed, modifying vl.h causes
> > > all files to be recompiled. This is of course the correct action, but
> > > it's a major slowdown for development too.
> >
> > There must be an option in the Makefile to disable the automatic
> > dependency check.
> >
> > > How should we split vl.h into smaller pieces? Give each device a
> > > header file, like m48t59? What about other stuff exported from vl.c?
> >
> > The net result is that you will have dozens of header files with only
> > one line in them as most devices only export one function.
> 
> I have another solution: include all architecture specific files from
> the main file. This actually makes the compilation faster and the
> resulting binary is smaller (maybe faster).

I it a solution? You always end up with the worst case of recompiling
everything now.

> Changing the architecture
> specific code needs no changes to vl.h, just a recompile of sun4m.c,
> but this is instantaneous on my machine. Automatic dependencies also
> handle this case. I guess some may find this style pretty ugly.

It is ugly. You basically re-invented gcc's -combine option but
without avoiding the namespace problem of a single file scope.



Thiemo