[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_exp
From: |
Beata Michalska |
Subject: |
Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion |
Date: |
Tue, 22 Oct 2019 16:49:15 +0100 |
Hi Andrew,
On Tue, 22 Oct 2019 at 14:43, Andrew Jones <address@hidden> wrote:
>
> On Mon, Oct 21, 2019 at 04:07:14PM +0100, Beata Michalska wrote:
> > Indeed, the patch got bit messed-up. Apologies for that as well.
> > I have been testing manually but I did try the test you have provided
> > and yes it fails - there is a slight problem with the case when qdict_in
> > is empty,but this can be easily solved still keeping the single loop.
> > Otherwise I have seen you have posted a new patchest so I guess we are
> > dropping the idea of refactoring ?
>
> Well, without a patch that applies, I couldn't really evaluate your
> proposal. And, TBH, I'd rather not hold this series up on a refactoring
> that doesn't provide measurable performance improvements, especially when
> it's not in a performance critical path. Indeed, I'd like to get this
> series merged as soon as possible, which is why I posted v6 with your
> visit_free() fix already.
>
> >
> > One more question: in case of querying a property which is not supported
> > by given cpu model - we are returning properties that are actually valid
> > (the test case for cortex-a15 and aarch64 prop).
> > Shouldn't we return an error there? I honestly must admit I do not know
> > what is the expected behaviour for the qmp query in such cases.
>
> We do generate an error for that case:
>
> (QEMU) query-cpu-model-expansion type=full model={"name":"cortex-a15"}
> {"return": {"model": {"name": "cortex-a15", "props": {"pmu": true}}}}
>
> (QEMU) query-cpu-model-expansion type=full
> model={"name":"cortex-a15","props":{"aarch64":false}}
> {"error": {"class": "GenericError", "desc": "Property '.aarch64' not found"}}
>
>
> If you have any more comments on the series, please send them right away.
> I'd like Peter to be able to merge this soon, and I understand that he's
> waiting on your review.
>
I think we can proceed with the v6 as it is.
Thanks a lot.
BR
Beata
> Thanks,
> drew
>
- [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, (continued)
- [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Andrew Jones, 2019/10/01
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Beata Michalska, 2019/10/15
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Andrew Jones, 2019/10/15
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Beata Michalska, 2019/10/15
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Beata Michalska, 2019/10/16
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Andrew Jones, 2019/10/16
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Beata Michalska, 2019/10/16
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Andrew Jones, 2019/10/16
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Beata Michalska, 2019/10/21
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion, Andrew Jones, 2019/10/22
- Re: [PATCH v5 1/9] target/arm/monitor: Introduce qmp_query_cpu_model_expansion,
Beata Michalska <=
[PATCH v5 2/9] tests: arm: Introduce cpu feature tests, Andrew Jones, 2019/10/01
[PATCH v5 3/9] target/arm: Allow SVE to be disabled via a CPU property, Andrew Jones, 2019/10/01
[PATCH v5 4/9] target/arm/cpu64: max cpu: Introduce sve<N> properties, Andrew Jones, 2019/10/01
[PATCH v5 6/9] target/arm/kvm64: max cpu: Enable SVE when available, Andrew Jones, 2019/10/01