Re: [PATCH] arm64: KVM: debug: Remove spurious inline attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marc Zyngier <marc.zyngier@xxxxxxx> writes:

> The debug trapping code is pretty heavy on the "inline" attribute,
> but most functions are actually referenced in the sysreg tables,
> making the inlining imposible.
>
> Removing the useless inline qualifier seems the right thing to do,
> having verified that the output code is similar.
>
> Cc: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>  arch/arm64/kvm/sys_regs.c | 58 +++++++++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 88adebf..eec3598 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -220,9 +220,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
>   * All writes will set the KVM_ARM64_DEBUG_DIRTY flag to ensure the
>   * hyp.S code switches between host and guest values in future.
>   */
> -static inline void reg_to_dbg(struct kvm_vcpu *vcpu,
> -			      struct sys_reg_params *p,
> -			      u64 *dbg_reg)
> +static void reg_to_dbg(struct kvm_vcpu *vcpu,
> +		       struct sys_reg_params *p,
> +		       u64 *dbg_reg)
>  {
>  	u64 val = p->regval;
>
> @@ -235,18 +235,18 @@ static inline void reg_to_dbg(struct kvm_vcpu *vcpu,
>  	vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
>  }
>
> -static inline void dbg_to_reg(struct kvm_vcpu *vcpu,
> -			      struct sys_reg_params *p,
> -			      u64 *dbg_reg)
> +static void dbg_to_reg(struct kvm_vcpu *vcpu,
> +		       struct sys_reg_params *p,
> +		       u64 *dbg_reg)
>  {
>  	p->regval = *dbg_reg;
>  	if (p->is_32bit)
>  		p->regval &= 0xffffffffUL;
>  }

Christoffer's "register keyword" comments not-withstanding I'd prefer to
keep the reg_to_dbg/dbg_to_reg functions as inline because they really
are just boilerplate helpers I didn't want to repeat in the actual
access functions - although if you've looked at the code I assume that
means GCC has been smart about it.

>
> -static inline bool trap_bvr(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *rd)
> +static bool trap_bvr(struct kvm_vcpu *vcpu,
> +		     struct sys_reg_params *p,
> +		     const struct sys_reg_desc *rd)
>  {
>  	u64 *dbg_reg = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
>
> @@ -280,15 +280,15 @@ static int get_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return 0;
>  }
>
> -static inline void reset_bvr(struct kvm_vcpu *vcpu,
> -			     const struct sys_reg_desc *rd)
> +static void reset_bvr(struct kvm_vcpu *vcpu,
> +		      const struct sys_reg_desc *rd)
>  {
>  	vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = rd->val;
>  }
>
> -static inline bool trap_bcr(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *rd)
> +static bool trap_bcr(struct kvm_vcpu *vcpu,
> +		     struct sys_reg_params *p,
> +		     const struct sys_reg_desc *rd)
>  {
>  	u64 *dbg_reg = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
>
> @@ -323,15 +323,15 @@ static int get_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return 0;
>  }
>
> -static inline void reset_bcr(struct kvm_vcpu *vcpu,
> -			     const struct sys_reg_desc *rd)
> +static void reset_bcr(struct kvm_vcpu *vcpu,
> +		      const struct sys_reg_desc *rd)
>  {
>  	vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] = rd->val;
>  }
>
> -static inline bool trap_wvr(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *rd)
> +static bool trap_wvr(struct kvm_vcpu *vcpu,
> +		     struct sys_reg_params *p,
> +		     const struct sys_reg_desc *rd)
>  {
>  	u64 *dbg_reg = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
>
> @@ -366,15 +366,15 @@ static int get_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return 0;
>  }
>
> -static inline void reset_wvr(struct kvm_vcpu *vcpu,
> -			     const struct sys_reg_desc *rd)
> +static void reset_wvr(struct kvm_vcpu *vcpu,
> +		      const struct sys_reg_desc *rd)
>  {
>  	vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] = rd->val;
>  }
>
> -static inline bool trap_wcr(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *rd)
> +static bool trap_wcr(struct kvm_vcpu *vcpu,
> +		     struct sys_reg_params *p,
> +		     const struct sys_reg_desc *rd)
>  {
>  	u64 *dbg_reg = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
>
> @@ -408,8 +408,8 @@ static int get_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return 0;
>  }
>
> -static inline void reset_wcr(struct kvm_vcpu *vcpu,
> -			     const struct sys_reg_desc *rd)
> +static void reset_wcr(struct kvm_vcpu *vcpu,
> +		      const struct sys_reg_desc *rd)
>  {
>  	vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] = rd->val;
>  }
> @@ -723,9 +723,9 @@ static bool trap_debug32(struct kvm_vcpu *vcpu,
>   * system is in.
>   */
>
> -static inline bool trap_xvr(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *rd)
> +static bool trap_xvr(struct kvm_vcpu *vcpu,
> +		     struct sys_reg_params *p,
> +		     const struct sys_reg_desc *rd)
>  {
>  	u64 *dbg_reg = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];

The rest all make sense. I wonder why I was being so inline happy?

Reviewed-by: Alex Bennée <alex.bennee@xxxxxxxxxx>

--
Alex Bennée
--
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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux