Re: [Qemu-devel] [PATCH 2/4] Allow ISA bus to be configured out

2015-06-30 Thread Andreas Färber
Am 30.06.2015 um 06:48 schrieb David Gibson:
 On Tue, Jun 30, 2015 at 01:44:10PM +1000, David Gibson wrote:
 On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote:
 Am 29.06.2015 um 07:36 schrieb David Gibson:
 Currently, the code to handle the legacy ISA bus is always included in
 qemu.  However there are lots of platforms that don't include ISA legacy
 devies, and quite a few that have never used ISA legacy devices at all.

 This patch allows the ISA bus code to be disabled in the configuration for
 platforms where it doesn't make sense.  For now, the default configs are
 adjusted to include ISA on all platforms including PCI (since
 CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several
 others which include ISA devices.  We may want to pare this down in future.

 PCI also allows to have a PCI-ISA bridge such as the i82378, which means
 that all PCI platforms can potentially obtain an actual ISA bus. So at
 least the commit message could use a makeover to avoid someone touching
 IDE to blindly disable the dependency.

 Sorry, I'm not quite following what you're getting at here.

 You'd also need to think about the qtests then:

 $ git grep i82378 -- tests/
 tests/endianness-test.c:{ ppc, g3beige, 0xfe00, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:{ ppc, bamboo, 0xe800, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:{ ppc64, mac99, 0xf200, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:  .bswap = true, .superio = i82378 },
 tests/endianness-test.c:{ sh4, r2d, 0xfe24, .superio =
 i82378 },
 tests/endianness-test.c:{ sh4eb, r2d, 0xfe24, .bswap = true,
 .superio = i82378 },

 Ah, good point.  I'll need to rework for that,
 
 Actually.. on second thoughts..
 
 CONFIG_I82378 already exists, and will break those tests of disabled.
 
 So while making the tests more robust against config changes would be
 a good thing in general, I don't think it's in the scope of what I'm
 trying to do here - making ISA configurable won't make these tests any
 more broken with nonstandard configs than they already are.

I am mainly saying that your commit message is misleading.

CONFIG_IDE_CORE is not the only reason that pci.mak needs
CONFIG_ISA_BUS=y, as you make it sound. The other problem is that we
don't have Kconfig yet, so we have no way of modeling that I82378
depends on ISA_BUS, therefore either pci.mak (as done here) or
ppc-softmmu.mak and ppc64-softmmu.mak need CONFIG_ISA_BUS=y. And due to
the qtest either sh4/sh4eb need that same dependency too, or the test
needs to be changed.

So please either revise the commit message when you resend, or you'll
need to repeat CONFIG_ISA_BUS=y for ppc, ppc64, sh4, sh4eb.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/4] Allow ISA bus to be configured out

2015-06-29 Thread Andreas Färber
Am 29.06.2015 um 07:36 schrieb David Gibson:
 Currently, the code to handle the legacy ISA bus is always included in
 qemu.  However there are lots of platforms that don't include ISA legacy
 devies, and quite a few that have never used ISA legacy devices at all.
 
 This patch allows the ISA bus code to be disabled in the configuration for
 platforms where it doesn't make sense.  For now, the default configs are
 adjusted to include ISA on all platforms including PCI (since
 CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several
 others which include ISA devices.  We may want to pare this down in future.

PCI also allows to have a PCI-ISA bridge such as the i82378, which means
that all PCI platforms can potentially obtain an actual ISA bus. So at
least the commit message could use a makeover to avoid someone touching
IDE to blindly disable the dependency.

You'd also need to think about the qtests then:

$ git grep i82378 -- tests/
tests/endianness-test.c:{ ppc, g3beige, 0xfe00, .bswap =
true, .superio = i82378 },
tests/endianness-test.c:{ ppc, bamboo, 0xe800, .bswap =
true, .superio = i82378 },
tests/endianness-test.c:{ ppc64, mac99, 0xf200, .bswap =
true, .superio = i82378 },
tests/endianness-test.c:  .bswap = true, .superio = i82378 },
tests/endianness-test.c:{ sh4, r2d, 0xfe24, .superio =
i82378 },
tests/endianness-test.c:{ sh4eb, r2d, 0xfe24, .bswap = true,
.superio = i82378 },

 
 This patch becomes more useful since b19c1c0 isa: remove isa_mem_base
 variable. since that removes a dependency on isa-bus.c from vga.c.
 
 Signed-off-by: David Gibson da...@gibson.dropbear.id.au
 ---
  default-configs/moxie-softmmu.mak | 1 +
  default-configs/pci.mak   | 1 +
  default-configs/sparc-softmmu.mak | 1 +
  default-configs/unicore32-softmmu.mak | 1 +
  hw/isa/Makefile.objs  | 2 +-
  5 files changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/default-configs/moxie-softmmu.mak 
 b/default-configs/moxie-softmmu.mak
 index 7e22863..e00d099 100644
 --- a/default-configs/moxie-softmmu.mak
 +++ b/default-configs/moxie-softmmu.mak
 @@ -1,5 +1,6 @@
  # Default configuration for moxie-softmmu
  
 +CONFIG_ISA_BUS=y
  CONFIG_MC146818RTC=y
  CONFIG_SERIAL=y
  CONFIG_SERIAL_ISA=y

I guess we can drop it for moxie ...

 diff --git a/default-configs/pci.mak b/default-configs/pci.mak
 index 7e10903..9f2b98c 100644
 --- a/default-configs/pci.mak
 +++ b/default-configs/pci.mak
 @@ -1,4 +1,5 @@
  CONFIG_PCI=y
 +CONFIG_ISA_BUS=y
  CONFIG_VIRTIO_PCI=y
  CONFIG_VIRTIO=y
  CONFIG_USB_UHCI=y
 diff --git a/default-configs/sparc-softmmu.mak 
 b/default-configs/sparc-softmmu.mak
 index ab796b3..004b0f4 100644
 --- a/default-configs/sparc-softmmu.mak
 +++ b/default-configs/sparc-softmmu.mak
 @@ -1,5 +1,6 @@
  # Default configuration for sparc-softmmu
  
 +CONFIG_ISA_BUS=y
  CONFIG_ECC=y
  CONFIG_ESP=y
  CONFIG_ESCC=y
 diff --git a/default-configs/unicore32-softmmu.mak 
 b/default-configs/unicore32-softmmu.mak
 index de38577..5f6c4a8 100644
 --- a/default-configs/unicore32-softmmu.mak
 +++ b/default-configs/unicore32-softmmu.mak
 @@ -1,4 +1,5 @@
  # Default configuration for unicore32-softmmu
 +CONFIG_ISA_BUS=y
  CONFIG_PUV3=y
  CONFIG_PTIMER=y
  CONFIG_PCKBD=y

... and for unicore32?

Regards,
Andreas

 diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
 index 9164556..fb37c55 100644
 --- a/hw/isa/Makefile.objs
 +++ b/hw/isa/Makefile.objs
 @@ -1,4 +1,4 @@
 -common-obj-y += isa-bus.o
 +common-obj-$(CONFIG_ISA_BUS) += isa-bus.o
  common-obj-$(CONFIG_APM) += apm.o
  common-obj-$(CONFIG_I82378) += i82378.o
  common-obj-$(CONFIG_PC87312) += pc87312.o

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)



Re: [Qemu-devel] [PATCH 2/4] Allow ISA bus to be configured out

2015-06-29 Thread David Gibson
On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote:
 Am 29.06.2015 um 07:36 schrieb David Gibson:
  Currently, the code to handle the legacy ISA bus is always included in
  qemu.  However there are lots of platforms that don't include ISA legacy
  devies, and quite a few that have never used ISA legacy devices at all.
  
  This patch allows the ISA bus code to be disabled in the configuration for
  platforms where it doesn't make sense.  For now, the default configs are
  adjusted to include ISA on all platforms including PCI (since
  CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several
  others which include ISA devices.  We may want to pare this down in future.
 
 PCI also allows to have a PCI-ISA bridge such as the i82378, which means
 that all PCI platforms can potentially obtain an actual ISA bus. So at
 least the commit message could use a makeover to avoid someone touching
 IDE to blindly disable the dependency.

Sorry, I'm not quite following what you're getting at here.

 You'd also need to think about the qtests then:
 
 $ git grep i82378 -- tests/
 tests/endianness-test.c:{ ppc, g3beige, 0xfe00, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:{ ppc, bamboo, 0xe800, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:{ ppc64, mac99, 0xf200, .bswap =
 true, .superio = i82378 },
 tests/endianness-test.c:  .bswap = true, .superio = i82378 },
 tests/endianness-test.c:{ sh4, r2d, 0xfe24, .superio =
 i82378 },
 tests/endianness-test.c:{ sh4eb, r2d, 0xfe24, .bswap = true,
 .superio = i82378 },

Ah, good point.  I'll need to rework for that,

-- 
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


pgpnYFmq2cQrL.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 2/4] Allow ISA bus to be configured out

2015-06-29 Thread David Gibson
On Tue, Jun 30, 2015 at 01:44:10PM +1000, David Gibson wrote:
 On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote:
  Am 29.06.2015 um 07:36 schrieb David Gibson:
   Currently, the code to handle the legacy ISA bus is always included in
   qemu.  However there are lots of platforms that don't include ISA legacy
   devies, and quite a few that have never used ISA legacy devices at all.
   
   This patch allows the ISA bus code to be disabled in the configuration for
   platforms where it doesn't make sense.  For now, the default configs are
   adjusted to include ISA on all platforms including PCI (since
   CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several
   others which include ISA devices.  We may want to pare this down in 
   future.
  
  PCI also allows to have a PCI-ISA bridge such as the i82378, which means
  that all PCI platforms can potentially obtain an actual ISA bus. So at
  least the commit message could use a makeover to avoid someone touching
  IDE to blindly disable the dependency.
 
 Sorry, I'm not quite following what you're getting at here.
 
  You'd also need to think about the qtests then:
  
  $ git grep i82378 -- tests/
  tests/endianness-test.c:{ ppc, g3beige, 0xfe00, .bswap =
  true, .superio = i82378 },
  tests/endianness-test.c:{ ppc, bamboo, 0xe800, .bswap =
  true, .superio = i82378 },
  tests/endianness-test.c:{ ppc64, mac99, 0xf200, .bswap =
  true, .superio = i82378 },
  tests/endianness-test.c:  .bswap = true, .superio = i82378 },
  tests/endianness-test.c:{ sh4, r2d, 0xfe24, .superio =
  i82378 },
  tests/endianness-test.c:{ sh4eb, r2d, 0xfe24, .bswap = true,
  .superio = i82378 },
 
 Ah, good point.  I'll need to rework for that,

Actually.. on second thoughts..

CONFIG_I82378 already exists, and will break those tests of disabled.

So while making the tests more robust against config changes would be
a good thing in general, I don't think it's in the scope of what I'm
trying to do here - making ISA configurable won't make these tests any
more broken with nonstandard configs than they already are.

-- 
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


pgpVOGL2tQmsC.pgp
Description: PGP signature


[Qemu-devel] [PATCH 2/4] Allow ISA bus to be configured out

2015-06-28 Thread David Gibson
Currently, the code to handle the legacy ISA bus is always included in
qemu.  However there are lots of platforms that don't include ISA legacy
devies, and quite a few that have never used ISA legacy devices at all.

This patch allows the ISA bus code to be disabled in the configuration for
platforms where it doesn't make sense.  For now, the default configs are
adjusted to include ISA on all platforms including PCI (since
CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several
others which include ISA devices.  We may want to pare this down in future.

This patch becomes more useful since b19c1c0 isa: remove isa_mem_base
variable. since that removes a dependency on isa-bus.c from vga.c.

Signed-off-by: David Gibson da...@gibson.dropbear.id.au
---
 default-configs/moxie-softmmu.mak | 1 +
 default-configs/pci.mak   | 1 +
 default-configs/sparc-softmmu.mak | 1 +
 default-configs/unicore32-softmmu.mak | 1 +
 hw/isa/Makefile.objs  | 2 +-
 5 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/default-configs/moxie-softmmu.mak 
b/default-configs/moxie-softmmu.mak
index 7e22863..e00d099 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for moxie-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
 CONFIG_SERIAL_ISA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 7e10903..9f2b98c 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -1,4 +1,5 @@
 CONFIG_PCI=y
+CONFIG_ISA_BUS=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO=y
 CONFIG_USB_UHCI=y
diff --git a/default-configs/sparc-softmmu.mak 
b/default-configs/sparc-softmmu.mak
index ab796b3..004b0f4 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for sparc-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_ECC=y
 CONFIG_ESP=y
 CONFIG_ESCC=y
diff --git a/default-configs/unicore32-softmmu.mak 
b/default-configs/unicore32-softmmu.mak
index de38577..5f6c4a8 100644
--- a/default-configs/unicore32-softmmu.mak
+++ b/default-configs/unicore32-softmmu.mak
@@ -1,4 +1,5 @@
 # Default configuration for unicore32-softmmu
+CONFIG_ISA_BUS=y
 CONFIG_PUV3=y
 CONFIG_PTIMER=y
 CONFIG_PCKBD=y
diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
index 9164556..fb37c55 100644
--- a/hw/isa/Makefile.objs
+++ b/hw/isa/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += isa-bus.o
+common-obj-$(CONFIG_ISA_BUS) += isa-bus.o
 common-obj-$(CONFIG_APM) += apm.o
 common-obj-$(CONFIG_I82378) += i82378.o
 common-obj-$(CONFIG_PC87312) += pc87312.o
-- 
2.4.3