[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 15/16] target/mips/cpu: Do not allow system-mode use without
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 15/16] target/mips/cpu: Do not allow system-mode use without input clock |
Date: |
Mon, 5 Oct 2020 18:25:02 +0200 |
On Tue, 29 Sep 2020 16:40:24 +0200
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 9/29/20 3:01 PM, Igor Mammedov wrote:
> > On Mon, 28 Sep 2020 19:15:38 +0200
> > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> >> Now than all QOM users provides the input clock, do not allow
> >> using a CPU core without its input clock connected on system-mode
> >> emulation. For user-mode, keep providing a fixed 200 MHz clock,
> >> as it used by the RDHWR instruction (see commit cdfcad788394).
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >> Cc: Igor Mammedov <imammedo@redhat.com>
> >>
> >> We need the qtest check for tests/qtest/machine-none-test.c
> >> which instanciate a CPU with the none machine. Igor, is it
> >> better to remove the MIPS targets from the test cpus_map[]?
it will help to pass test but,
> >
> > I don't get idea, could you rephrase/elaborate?
>
> cpu_class_init sets:
>
> /*
> * Reason: CPUs still need special care by board code: wiring up
> * IRQs, adding reset handlers, halting non-first CPUs, ...
> */
> dc->user_creatable = false;
>
> but the CPUs are created via another path in vl.c:
>
> current_machine->cpu_type = parse_cpu_option(cpu_option);
>
> The machine-none-test assumes CPU objects are user-creatable.
user_creatable -> able to create using -device,
so test doesn't make this assumption.
problem is that enforcing would make machine-none not usable
for probing CPU features which is used by mgmt,
so that would break.
>
> With this patch I enforce MIPS CPU to have an input clock
> connected, so they are not user-creatable anymore.
>
> I tried this code ...:
>
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -229,7 +229,11 @@ static const TypeInfo mips_cpu_type_info = {
> static void mips_cpu_cpudef_class_init(ObjectClass *oc, void *data)
> {
> MIPSCPUClass *mcc = MIPS_CPU_CLASS(oc);
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> mcc->cpu_def = data;
> + /* Reason: clock need to be wired up */
> + dc->user_creatable = false;
> }
>
> ... but it is ignored, the CPU can still be created.
>
> Not sure how to handle this.
>
> >
> >> ---
> >> target/mips/cpu.c | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> >> index 2f75216c324..cc4ee75af30 100644
> >> --- a/target/mips/cpu.c
> >> +++ b/target/mips/cpu.c
> >> @@ -25,6 +25,7 @@
> >> #include "kvm_mips.h"
> >> #include "qemu/module.h"
> >> #include "sysemu/kvm.h"
> >> +#include "sysemu/qtest.h"
> >> #include "exec/exec-all.h"
> >> #include "hw/qdev-clock.h"
> >> #include "hw/qdev-properties.h"
> >> @@ -159,11 +160,18 @@ static void mips_cpu_realizefn(DeviceState *dev,
> >> Error **errp)
> >> Error *local_err = NULL;
> >>
> >> if (!clock_get(cs->clock)) {
> >> +#ifdef CONFIG_USER_ONLY
> >> /*
> >> * Initialize the frequency to 200MHz in case
> >> * the clock remains unconnected.
> >> */
> >> clock_set_hz(cs->clock, 200000000);
> >> +#else
> >> + if (!qtest_enabled()) {
> >> + error_setg(errp, "CPU clock must be connected to a clock
> >> source");
> >> + return;
> >> + }
> >> +#endif
> >> }
> >> mips_cpu_clk_update(cs);
> >>
> >
>