diff mbox series

[3/5] target/arm: Introduce read_sys_reg32 for kvm32

Message ID 20181024113709.16599-4-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: KVM vs ARMISARegisters | expand

Commit Message

Richard Henderson Oct. 24, 2018, 11:37 a.m. UTC
Assert that the value to be written is the correct size.
No change in functionality here, just mirroring the same
function from kvm64.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/kvm32.c | 41 ++++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

-- 
2.17.2

Comments

Richard Henderson Oct. 24, 2018, 12:49 p.m. UTC | #1
On 10/24/18 12:37 PM, Richard Henderson wrote:
> -    int i, ret, fdarray[3];

> +    int i, err = 0, fdarray[3];


Bah.  The bit about having compile-tested for arm32 was fake news -- "i" is now
unused.  Ho hum.

> @@ -77,16 +69,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)

>       */

>      ahcf->dtb_compatible = "arm,arm-v7";

>  

> -    for (i = 0; i < ARRAY_SIZE(idregs); i++) {

> -        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);

> -        if (ret) {

> -            break;

> -        }

> -    }

> +    err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0));

> +    err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0));

> +    err |= read_sys_reg32(fdarray[2], &mvfr1,

> +                          KVM_REG_ARM | KVM_REG_SIZE_U32 |

> +                          KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1);



r~
Peter Maydell Nov. 2, 2018, 2:30 p.m. UTC | #2
On 24 October 2018 at 12:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Assert that the value to be written is the correct size.

> No change in functionality here, just mirroring the same

> function from kvm64.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/kvm32.c | 41 ++++++++++++++++-------------------------

>  1 file changed, 16 insertions(+), 25 deletions(-)


> -    struct kvm_one_reg idregs[] = {

> -        {

> -            .id = KVM_REG_ARM | KVM_REG_SIZE_U32

> -            | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0),

> -            .addr = (uintptr_t)&midr,

> -        },

> -        {

> -            .id = KVM_REG_ARM | KVM_REG_SIZE_U32

> -            | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0),

> -            .addr = (uintptr_t)&id_pfr0,

> -        },

> -        {

> -            .id = KVM_REG_ARM | KVM_REG_SIZE_U32

> -            | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,

> -            .addr = (uintptr_t)&mvfr1,

> -        },

> -    };

>

>      if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {

>          return false;

> @@ -77,16 +69,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)

>       */

>      ahcf->dtb_compatible = "arm,arm-v7";

>

> -    for (i = 0; i < ARRAY_SIZE(idregs); i++) {

> -        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);

> -        if (ret) {

> -            break;

> -        }

> -    }

> +    err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0));

> +    err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0));



Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

with the declaration of 'i' removed.

(Confusingly, ENCODE_CP_REG and ARM_CP15_REG32 take the
op1/crn/crm/op2 arguments in different orders.)

thanks
-- PMM
Richard Henderson Nov. 2, 2018, 2:37 p.m. UTC | #3
On 11/2/18 2:30 PM, Peter Maydell wrote:
> (Confusingly, ENCODE_CP_REG and ARM_CP15_REG32 take the

> op1/crn/crm/op2 arguments in different orders.)


As Shaggy said, "It wasn't me".  ;-)


r~
diff mbox series

Patch

diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index 0f1e94c7b5..da08f7aab8 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -28,6 +28,14 @@  static inline void set_feature(uint64_t *features, int feature)
     *features |= 1ULL << feature;
 }
 
+static int read_sys_reg32(int fd, uint32_t *pret, uint64_t id)
+{
+    struct kvm_one_reg idreg = { .id = id, .addr = (uintptr_t)pret };
+
+    assert((id & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U32);
+    return ioctl(fd, KVM_GET_ONE_REG, &idreg);
+}
+
 bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 {
     /* Identify the feature bits corresponding to the host CPU, and
@@ -35,9 +43,10 @@  bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      * we have to create a scratch VM, create a single CPU inside it,
      * and then query that CPU for the relevant ID registers.
      */
-    int i, ret, fdarray[3];
+    int i, err = 0, fdarray[3];
     uint32_t midr, id_pfr0, mvfr1;
     uint64_t features = 0;
+
     /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
      * we know these will only support creating one kind of guest CPU,
      * which is its preferred CPU type.
@@ -47,23 +56,6 @@  bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
         QEMU_KVM_ARM_TARGET_NONE
     };
     struct kvm_vcpu_init init;
-    struct kvm_one_reg idregs[] = {
-        {
-            .id = KVM_REG_ARM | KVM_REG_SIZE_U32
-            | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0),
-            .addr = (uintptr_t)&midr,
-        },
-        {
-            .id = KVM_REG_ARM | KVM_REG_SIZE_U32
-            | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0),
-            .addr = (uintptr_t)&id_pfr0,
-        },
-        {
-            .id = KVM_REG_ARM | KVM_REG_SIZE_U32
-            | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
-            .addr = (uintptr_t)&mvfr1,
-        },
-    };
 
     if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
         return false;
@@ -77,16 +69,15 @@  bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      */
     ahcf->dtb_compatible = "arm,arm-v7";
 
-    for (i = 0; i < ARRAY_SIZE(idregs); i++) {
-        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
-        if (ret) {
-            break;
-        }
-    }
+    err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0));
+    err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0));
+    err |= read_sys_reg32(fdarray[2], &mvfr1,
+                          KVM_REG_ARM | KVM_REG_SIZE_U32 |
+                          KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1);
 
     kvm_arm_destroy_scratch_host_vcpu(fdarray);
 
-    if (ret) {
+    if (err < 0) {
         return false;
     }