On Mon, 29 Jun 2015 11:55:08 +0200
Laurent Vivier <lviv...@redhat.com> wrote:
>
>
> 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 <lviv...@redhat.com> wrote:
> >>
> >>>
> >>>
> >>> On 29/06/2015 07:36, David Gibson wrote:
> >>>> The "info irq" and "info pic" HMP commands are available on some, but not
> >>>> all targets, and what they do isn't terribly consistent. For SPARC and
> >>>> LM32 they do something platform specific, but for x86, powerpc, and MIPS
> >>>> they print some information from the i8259 (and only the i8259) interrupt
> >>>> controller.
> >>>>
> >>>> It's debatable whether these commands are any use at all, and we should
> >>>> probably make better, qdev aware ways of getting information from a
> >>>> machines PICs. However, those don't exist yet, so on x86 it's at least
> >>>> potentially useful to have these HMP commands. I can't speak for MIPS.
> >>>>
> >>>> For ppc, though, the i8259, if it exists at all, is usually just a
> >>>> secondary controller for legacy ISA. The only case where i8259 is the
> >>>> main system PIC on ppc is for the ancient and little-used PReP platform.
> >>>>
> >>>> So, even without QOM-ish replacement, the info pic and info irq HMP
> >>>> commands have no value on ppc.
> >>>>
> >>>> This patch, therefore, disables these commands for ppc targets. This
> >>>> will
> >>>> allow ppc builds which don't include PReP to not include ISA bus support
> >>>> either.
> >>>>
> >>>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> >>>> ---
> >>>> monitor.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> 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)
+1
I like that idea, sounds like a good compromise.
Thomas
