On 21.09.2012, at 07:44, Paul Mackerras wrote:
> This enables userspace to get and set various SPRs (special-purpose
> registers) using the KVM_[GS]ET_ONE_REG ioctls. With this, userspace
> can get and set all the SPRs that are part of the guest state, either
> through the KVM_[GS]ET_REGS ioctls, the KVM_[GS]ET_SREGS ioctls, or
> the KVM_[GS]ET_ONE_REG ioctls.
>
> The SPRs that are added here are:
>
> - DABR: Data address breakpoint register
> - DSCR: Data stream control register
> - PURR: Processor utilization of resources register
> - SPURR: Scaled PURR
> - DAR: Data address register
> - DSISR: Data storage interrupt status register
> - AMR: Authority mask register
> - UAMOR: User authority mask override register
> - MMCR0, MMCR1, MMCRA: Performance monitor unit control registers
> - PMC1..PMC8: Performance monitor unit counter registers
>
> In order to reduce code duplication between PR and HV KVM code, this
> moves the kvm_vcpu_ioctl_[gs]et_one_reg functions into book3s.c and
> centralizes the copying between user and kernel space there. The
> registers that are handled differently between PR and HV, and those
> that exist only in one flavor, are handled in kvmppc_[gs]et_one_reg()
> functions that are specific to each flavor.
>
> Signed-off-by: Paul Mackerras <paulus@xxxxxxxxx>
> ---
> v3: handle DAR and DSISR, plus copy to/from userspace, in common code
>
> Documentation/virtual/kvm/api.txt | 19 +++++++++
> arch/powerpc/include/asm/kvm.h | 21 ++++++++++
> arch/powerpc/include/asm/kvm_ppc.h | 7 ++++
> arch/powerpc/kvm/book3s.c | 74 +++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv.c | 76 ++++++++++++++++++++++++++++++------
> arch/powerpc/kvm/book3s_pr.c | 23 ++++++-----
> 6 files changed, 196 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 76a07a6..e4a2067 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1740,6 +1740,25 @@ registers, find a list below:
> PPC | KVM_REG_PPC_IAC4 | 64
> PPC | KVM_REG_PPC_DAC1 | 64
> PPC | KVM_REG_PPC_DAC2 | 64
> + PPC | KVM_REG_PPC_DABR | 64
> + PPC | KVM_REG_PPC_DSCR | 64
> + PPC | KVM_REG_PPC_PURR | 64
> + PPC | KVM_REG_PPC_SPURR | 64
> + PPC | KVM_REG_PPC_DAR | 64
> + PPC | KVM_REG_PPC_DSISR | 32
> + PPC | KVM_REG_PPC_AMR | 64
> + PPC | KVM_REG_PPC_UAMOR | 64
> + PPC | KVM_REG_PPC_MMCR0 | 64
> + PPC | KVM_REG_PPC_MMCR1 | 64
> + PPC | KVM_REG_PPC_MMCRA | 64
> + PPC | KVM_REG_PPC_PMC1 | 32
> + PPC | KVM_REG_PPC_PMC2 | 32
> + PPC | KVM_REG_PPC_PMC3 | 32
> + PPC | KVM_REG_PPC_PMC4 | 32
> + PPC | KVM_REG_PPC_PMC5 | 32
> + PPC | KVM_REG_PPC_PMC6 | 32
> + PPC | KVM_REG_PPC_PMC7 | 32
> + PPC | KVM_REG_PPC_PMC8 | 32
>
> 4.69 KVM_GET_ONE_REG
>
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> index 3c14202..9557576 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -338,5 +338,26 @@ struct kvm_book3e_206_tlb_params {
> #define KVM_REG_PPC_IAC4 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
> #define KVM_REG_PPC_DAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
> #define KVM_REG_PPC_DAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
> +#define KVM_REG_PPC_DABR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8)
> +#define KVM_REG_PPC_DSCR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x9)
> +#define KVM_REG_PPC_PURR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xa)
> +#define KVM_REG_PPC_SPURR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xb)
> +#define KVM_REG_PPC_DAR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc)
> +#define KVM_REG_PPC_DSISR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0xd)
> +#define KVM_REG_PPC_AMR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xe)
> +#define KVM_REG_PPC_UAMOR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xf)
> +
> +#define KVM_REG_PPC_MMCR0 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
> +#define KVM_REG_PPC_MMCR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
> +#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
> +
> +#define KVM_REG_PPC_PMC1 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x18)
> +#define KVM_REG_PPC_PMC2 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x19)
> +#define KVM_REG_PPC_PMC3 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1a)
> +#define KVM_REG_PPC_PMC4 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1b)
> +#define KVM_REG_PPC_PMC5 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1c)
> +#define KVM_REG_PPC_PMC6 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1d)
> +#define KVM_REG_PPC_PMC7 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1e)
> +#define KVM_REG_PPC_PMC8 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1f)
>
> #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 2c94cb3..6002b0a 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -196,6 +196,11 @@ static inline u32 kvmppc_set_field(u64 inst, int msb, int lsb, int value)
> return r;
> }
>
> +union kvmppc_one_reg {
> + u32 wval;
> + u64 dval;
Phew. Is this guaranteed to always pad on the right, rather than left?
> +};
> +
> void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
> int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
>
> @@ -204,6 +209,8 @@ int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
>
> int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg);
> int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg);
> +int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id, union kvmppc_one_reg *);
> +int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id, union kvmppc_one_reg *);
>
> int kvm_vcpu_get_vpa_info(struct kvm_vcpu *vcpu, struct kvm_ppc_vpa *vpa);
> int kvm_vcpu_set_vpa_info(struct kvm_vcpu *vcpu, struct kvm_ppc_vpa *vpa);
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index e946665..6d1306c 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -485,6 +485,80 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
> return -ENOTSUPP;
> }
>
> +static int one_reg_size(u64 id)
> +{
> + id = (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT;
> + return 1 << id;
> +}
Rusty has a patch to export this as a generic function for all kvm targets. Check out
http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/97553
I actually like static inlines better, but we really should keep this code on top level :).
> +
> +int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> +{
> + int r;
> + union kvmppc_one_reg val;
> + int size;
> +
> + size = one_reg_size(reg->id);
> + if (size > sizeof(val))
> + return -EINVAL;
> +
> + r = kvmppc_get_one_reg(vcpu, reg->id, &val);
> +
> + if (r == -EINVAL) {
> + r = 0;
> + switch (reg->id) {
> + case KVM_REG_PPC_DAR:
> + val.dval = vcpu->arch.shared->dar;
Hmm. I would like to keep the amount of places who need to manually know about the size of a register as low as possible. We do know the size of the register already on in the REG id and in sizeof(register), right?
I think there will be valid scenarios for them to be of different size. For example when a register suddenly increases in size. But the assignment should always happen on the size the register id tells us.
So how about something like
#define kvmppc_set_reg(id, val, reg) { \
switch (one_reg_size(id)) { \
case 4: val.wval = reg; break; \
case 8: val.dval = reg; break; \
default: BUG(); \
} \
}
case KVM_REG_PPC_DAR:
kvmppc_set_reg(reg->id, &val, vcpu->arch.shared->dar);
break;
Maybe you can come up with something even easier though :).
Btw, with such a scheme in place, we wouldn't even need the union. We could just reserve a char array and cast it in the setter/getter functions.
Alex
--
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
