On Thu, Mar 29, 2012 at 1:17 AM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> Rather than just making all of c9 read-zero/write-discard, this changes
> it to the explicit profiling registers we need. This is a start for the
> future implementation were we actually implement performance monitoring,
> and makes sure we're not discarding important things.
>
> Signed-off-by: Rusty Russell <rusty.russell@xxxxxxxxxx>
> ---
> arch/arm/kvm/emulate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> index aec1b6e..4bdab8f 100644
> --- a/arch/arm/kvm/emulate.c
> +++ b/arch/arm/kvm/emulate.c
> @@ -238,6 +238,64 @@ static bool read_l2ctlr(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +static bool read_pmcr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + unsigned long arg)
> +{
> + u32 imp, idcode, num;
> +
> + imp = (vcpu->arch.cp15[c0_MIDR] & 0xFF000000) >> 24;
> + idcode = (vcpu->arch.cp15[c0_MIDR] & 0x00000FF0) >> 4;
where does it say that idcode is always te same as MIDR?
> + /* No counters. */
> + num = 0;
> +
> + /* Other bits are at reset value. */
what's the point of writing anything below then? I would assume that
you can have really weird behavior if you read something different
from what you once wrote, shouldn't you read num from the vcpu value
with the bit-filter below?
> + *vcpu_reg(vcpu, p->Rt1) = (imp << 24) | (idcode << 16) | (num << 11);
> + return true;
> +}
> +
> +/* FIXME: We ignore them enabling performance monitoring. */
if this is a FIXME, then how is it eventually going to be fixed?
> +static bool write_pmcr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + unsigned long arg)
> +{
> + u32 val = *vcpu_reg(vcpu, p->Rt1);
> +
> + kvm_debug("pmcr write:%s%s%s%s%s%s\n",
> + val & (1 << 5) ? " DP" : "",
> + val & (1 << 4) ? " X" : "",
> + val & (1 << 3) ? " D" : "",
> + val & (1 << 2) ? " C" : "",
> + val & (1 << 1) ? " P" : "",
> + val & (1 << 0) ? " E" : "");
> + return true;
> +}
> +
> +static bool read_pmcntenclr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + unsigned long arg)
> +{
> + /* Cycle counter is off, there are no others. */
> + *vcpu_reg(vcpu, p->Rt1) = 0;
> + return true;
> +}
> +
> +static bool write_pmcntenclr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + unsigned long arg)
> +{
> + /* Writing a 1 means disable a counter. That's cool. */
> + return true;
> +}
> +
> +static bool write_pmintenclr(struct kvm_vcpu *vcpu,
> + const struct coproc_params *p,
> + unsigned long arg)
> +{
> + /* Writing a 1 means disable an overflow interrupt. That's cool. */
> + return true;
> +}
> +
> static bool access_cp15_reg(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> unsigned long cp15_reg)
> @@ -279,13 +337,18 @@ struct coproc_emulate {
> static const struct coproc_emulate coproc_emulate[] = {
> /*
> * L2CTLR access (guest wants to know #CPUs).
> - *
> - * FIXME: Hack Alert: Read zero as default case.
> */
> { CRn( 9), CRm( 0), Op1( 1), Op2( 2), is32, READ, read_l2ctlr},
> { CRn( 9), CRm(DF), Op1(DF), Op2(DF), is32, WRITE, ignore_write},
> { CRn( 9), CRm(DF), Op1(DF), Op2(DF), is32, READ, read_zero},
>
> + /* Guest reads/writes PMU, assuming there will be one. */
> + { CRn( 9), CRm(12), Op1( 0), Op2( 0), is32, READ, read_pmcr},
> + { CRn( 9), CRm(12), Op1( 0), Op2( 0), is32, WRITE, write_pmcr},
> + { CRn( 9), CRm(12), Op1( 0), Op2( 2), is32, READ, read_pmcntenclr},
> + { CRn( 9), CRm(12), Op1( 0), Op2( 2), is32, WRITE, write_pmcntenclr},
> + { CRn( 9), CRm(14), Op1( 0), Op2( 2), is32, WRITE, write_pmintenclr},
> +
won't all the DF versions above "eat" these calls? Should they just go
away or are there still default cases to catch in which case the
comment about reading zero should perhaps be modified to specifically
mention these cases?
> /*
> * These CRn == 10 entries may not need to exist - if we can
> * ignore guest attempts to tamper with TLB lockdowns then it
Thanks,
Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
