Re: [Qemu-devel] [PATCH 4/4] Disable "info irq" and "info pic" for target-ppc

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Jun 29, 2015 at 01:02:50PM +0200, Alexander Graf wrote:
> On 06/29/15 12:43, Andreas Färber wrote:
> >Am 29.06.2015 um 12:36 schrieb Andreas Färber:
> >>Am 29.06.2015 um 12:22 schrieb Laurent Vivier:
> >>>On 29/06/2015 12:06, Andreas Färber wrote:
> >>>>Am 29.06.2015 um 11:55 schrieb Laurent Vivier:
> >>>>>On 29/06/2015 11:51, Laurent Vivier wrote:
> >>>>>>On 29/06/2015 11:30, Thomas Huth wrote:
> >>>>>>>On Mon, 29 Jun 2015 09:52:56 +0200
> >>>>>>>Laurent Vivier <lvivier@redhat.com> wrote:
> >>>>>>>>On 29/06/2015 07:36, David Gibson wrote:
> >>>>>>>>>diff --git a/monitor.c b/monitor.c
> >>>>>>>>>index aeea2b5..8c56bfa 100644
> >>>>>>>>>--- a/monitor.c
> >>>>>>>>>+++ b/monitor.c
> >>>>>>>>>@@ -2573,7 +2573,7 @@ static mon_cmd_t info_cmds[] = {
> >>>>>>>>>          .help       = "show the command line history",
> >>>>>>>>>          .mhandler.cmd = hmp_info_history,
> >>>>>>>>>      },
> >>>>>>>>>-#if defined(TARGET_I386) || defined(TARGET_PPC) || defined(TARGET_MIPS) || \
> >>>>>>>>>+#if defined(TARGET_I386) || defined(TARGET_MIPS) || \
> >>>>>>>>>      defined(TARGET_LM32) || (defined(TARGET_SPARC) && !defined(TARGET_SPARC64))
> >>>>>>>>>      {
> >>>>>>>>>          .name       = "irq",
> >>>>>>>>>
> >>>>>>>>Perhaps we can a use a "#if defined(CONFIG_I8259) ||
> >>>>>>>>defined(CONFIG_LM32) || (defined(TARGE_SPARC) &&
> >>>>>>>>!defined(TARGET_SPARC64))" instead, so we keep the command for PReP ?
> >>>>>>>AFAIK this currently won't work since CONFIG_I8259 is only defined for
> >>>>>>>the Makefiles, but not for the C pre-processor :-(
> >>>>>>Yes, I see that afterward, but ...
> >>>>>>
> >>>>>>>So unless somebody fixes that first, I think David's approach is the
> >>>>>>>only practicable solution right now.
> >>>>>>if you add "config-devices.h" in GENERATED_HEADERS in Makefile.target,
> >>>>>>and include "config-devices.h" in monitor.c, it works (all PREP
> >>>>>>dependencies in default-configs/ppc64-softmmu.mak must be removed too)
> >>>>>>
> >>>>>>But does this change acceptable for a tiny improvement ?
> >>>>>In fine, I think we can also do like for sparc:
> >>>>>
> >>>>>defined(TARGET_PPC) && !defined(TARGET_PPC64)
> >>>>Alex specifically requested PReP to be made available in ppc64, too.
> >>>Thank you Andreas.
> >>>
> >>>But why ? (I didn't find the answer with google, a link can be helpful).
> >http://git.qemu-project.org/?p=qemu.git;a=commit;h=acbb090b2400f627a801074c4e3e006c7501bb26
> >
> >(found by looking at the ppc64-softmmu.mak Git history)
> >
> >Judging by Markus as the reporter, I assume it was a tree-wide analysis
> >that came up with this inconsistency, which I was then asked to fix this
> >way.
> 
> Yes, it's consistency. All 64bit targets allow to run their 32bit machine
> types as well. Whether this is a sensible thing to do or not is a different
> discussion that (if we want to) we need to do outside of the scope of this
> mail thread.
> 
> As far as this patch goes, you get a clear nack from me, as it's a
> regression for the prep target. Please just QOM'ify the interrupt

"just"!?!

> controller, add an interfact that allows you to query the irq stats and then
> loop through all devices searching for that interface in the object tree.
> That way we should be able to get rid of all #ifdefs in that particular code
> and enable new irq controllers to expose their stats easily.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]