On Thu, Jul 12, 2012 at 7:54 AM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> We don't actually use the arg any more, and we're about to expand
> the struct, so give the callbacks direct access.
>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> ---
> arch/arm/kvm/coproc.c | 63 ++++++++++++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 2f8e6a0..4f06dd2 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -39,6 +39,19 @@ struct coproc_params {
> bool is_write;
> };
>
> +struct coproc_reg {
> + unsigned long CRn;
> + unsigned long CRm;
> + unsigned long Op1;
> + unsigned long Op2;
> +
> + bool is_64;
> +
> + bool (*access)(struct kvm_vcpu *,
> + const struct coproc_params *,
> + const struct coproc_reg *);
> +};
> +
> static void print_cp_instr(const struct coproc_params *p)
> {
> /* Look, we even formatted it for you to paste into the table! */
> @@ -79,9 +92,9 @@ int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> static bool ignore_write(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + bool trace)
> {
> - if (arg)
> + if (trace)
> trace_kvm_emulate_cp15_imp(p->Op1, p->Rt1, p->CRn, p->CRm,
> p->Op2, p->is_write);
> return true;
> @@ -89,9 +102,9 @@ static bool ignore_write(struct kvm_vcpu *vcpu,
>
> static bool read_zero(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + bool trace)
> {
> - if (arg)
> + if (trace)
> trace_kvm_emulate_cp15_imp(p->Op1, p->Rt1, p->CRn, p->CRm,
> p->Op2, p->is_write);
> *vcpu_reg(vcpu, p->Rt1) = 0;
> @@ -100,13 +113,13 @@ static bool read_zero(struct kvm_vcpu *vcpu,
>
> /* A15 TRM 4.3.48: R/O WI. */
> static bool access_l2ctlr(struct kvm_vcpu *vcpu,
> - const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_params *p,
> + const struct coproc_reg *r)
> {
> u32 l2ctlr, ncores;
>
> if (p->is_write)
> - return ignore_write(vcpu, p, 0);
> + return ignore_write(vcpu, p, false);
>
> asm volatile("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> l2ctlr &= ~(3 << 24);
> @@ -119,10 +132,10 @@ static bool access_l2ctlr(struct kvm_vcpu *vcpu,
> /* A15 TRM 4.3.49: R/O WI (even if NSACR.NS_L2ERR, a write of 1 is ignored). */
> static bool access_l2ectlr(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_reg *r)
> {
> if (p->is_write)
> - return ignore_write(vcpu, p, 0);
> + return ignore_write(vcpu, p, false);
>
> *vcpu_reg(vcpu, p->Rt1) = 0;
> return true;
> @@ -131,22 +144,22 @@ static bool access_l2ectlr(struct kvm_vcpu *vcpu,
> /* A15 TRM 4.3.60: R/O. */
> static bool access_cbar(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_reg *r)
> {
> if (p->is_write)
> return false;
> - return read_zero(vcpu, p, 0);
> + return read_zero(vcpu, p, false);
> }
>
> /* A15 TRM 4.3.28: RO WI */
> static bool access_actlr(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_reg *r)
> {
> u32 actlr;
>
> if (p->is_write)
> - return ignore_write(vcpu, p, 0);
> + return ignore_write(vcpu, p, false);
>
> asm volatile("mrc p15, 0, %0, c1, c0, 1\n" : "=r" (actlr));
> /* Make the SMP bit consistent with the guest configuration */
> @@ -162,7 +175,7 @@ static bool access_actlr(struct kvm_vcpu *vcpu,
> /* See note at ARM ARM B1.14.4 */
> static bool access_dcsw(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_reg *r)
> {
> u32 val;
>
> @@ -199,12 +212,12 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> */
> static bool pm_fake(struct kvm_vcpu *vcpu,
> const struct coproc_params *p,
> - unsigned long arg)
> + const struct coproc_reg *r)
> {
> if (p->is_write)
> - return ignore_write(vcpu, p, arg);
> + return ignore_write(vcpu, p, false);
> else
> - return read_zero(vcpu, p, arg);
> + return read_zero(vcpu, p, false);
> }
>
> #define access_pmcr pm_fake
> @@ -221,20 +234,6 @@ static bool pm_fake(struct kvm_vcpu *vcpu,
> #define access_pmintenset pm_fake
> #define access_pmintenclr pm_fake
>
> -struct coproc_reg {
> - unsigned long CRn;
> - unsigned long CRm;
> - unsigned long Op1;
> - unsigned long Op2;
> -
> - bool is_64;
> -
> - bool (*access)(struct kvm_vcpu *,
> - const struct coproc_params *,
> - unsigned long);
> - unsigned long arg;
> -};
> -
> #define CRn(_x) .CRn = _x
> #define CRm(_x) .CRm = _x
> #define Op1(_x) .Op1 = _x
> @@ -336,7 +335,7 @@ static int emulate_cp15(struct kvm_vcpu *vcpu,
> r = find_reg(params, cp15_regs, ARRAY_SIZE(cp15_regs));
>
> if (likely(r)) {
> - if (likely(r->access(vcpu, params, r->arg))) {
> + if (likely(r->access(vcpu, params, r))) {
> /* Skip instruction, since it was emulated */
> int instr_len = ((vcpu->arch.hsr >> 25) & 1) ? 4 : 2;
> *vcpu_pc(vcpu) += instr_len;
I'll apply this one (why not), but why do we have this code if we
never call it. I forget the original intention behind the trace point,
but wouldn't it be useful now to trace all the cp15 accesses in the
emulate_cp15 function and be rid of the "bool trace" argument?
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
