diff mbox series

[RFC,v2,2/7] target/arm: Allow KVM device address overwriting

Message ID 1526222114-5324-3-git-send-email-eric.auger@redhat.com
State New
Headers show
Series KVM/ARM: Relax the max 123 vcpus limitation along with KVM GICv3 | expand

Commit Message

Eric Auger May 13, 2018, 2:35 p.m. UTC
for KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute, the attribute
data pointed to by kvm_device_attr.addr is a OR of the
redistributor region address and other fields such as the index
of the redistributor region and the number of redistributors the
region can contain.

The existing machine init done notifier framework sets the address
field to the actual address of the device and does not allow to OR
this value with other fields.

This patch extends the KVMDevice struct with a new kda_addr_fixup
member. Its value is passed at registration time and OR'ed with the
resolved address on kvm_arm_set_device_addr().

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/intc/arm_gic_kvm.c       |  4 ++--
 hw/intc/arm_gicv3_its_kvm.c |  2 +-
 hw/intc/arm_gicv3_kvm.c     |  4 ++--
 target/arm/kvm.c            | 10 +++++++++-
 target/arm/kvm_arm.h        |  3 ++-
 5 files changed, 16 insertions(+), 7 deletions(-)

Comments

Peter Maydell May 22, 2018, 12:33 p.m. UTC | #1
On 13 May 2018 at 15:35, Eric Auger <eric.auger@redhat.com> wrote:
> for KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute, the attribute
> data pointed to by kvm_device_attr.addr is a OR of the
> redistributor region address and other fields such as the index
> of the redistributor region and the number of redistributors the
> region can contain.
>
> The existing machine init done notifier framework sets the address
> field to the actual address of the device and does not allow to OR
> this value with other fields.
>
> This patch extends the KVMDevice struct with a new kda_addr_fixup
> member. Its value is passed at registration time and OR'ed with the
> resolved address on kvm_arm_set_device_addr().


> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 1e23640..4050017 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -34,6 +34,7 @@ int kvm_arm_vcpu_init(CPUState *cs);
>   * @group: device control API group for setting addresses
>   * @attr: device control API address type
>   * @dev_fd: device control device file descriptor (or -1 if not supported)
> + * @addr_fixup: value to be OR'ed with resolved address

I think addr_fixup is a bit vague; how about
 addr_fixed_bits ?
(here and in the function argument name). Or something else if
somebody has a better idea.

>   *
>   * Remember the memory region @mr, and when it is mapped by the
>   * machine model, tell the kernel that base address using the
> @@ -45,7 +46,7 @@ int kvm_arm_vcpu_init(CPUState *cs);
>   * address at the point where machine init is complete.
>   */
>  void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
> -                             uint64_t attr, int dev_fd);
> +                             uint64_t attr, int dev_fd, uint64_t addr_fixup);
>
>  /**
>   * kvm_arm_init_cpreg_list:
> --
> 2.5.5

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

thanks
-- PMM
Peter Maydell May 22, 2018, 12:44 p.m. UTC | #2
On 22 May 2018 at 13:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 May 2018 at 15:35, Eric Auger <eric.auger@redhat.com> wrote:
>> for KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute, the attribute
>> data pointed to by kvm_device_attr.addr is a OR of the
>> redistributor region address and other fields such as the index
>> of the redistributor region and the number of redistributors the
>> region can contain.
>>
>> The existing machine init done notifier framework sets the address
>> field to the actual address of the device and does not allow to OR
>> this value with other fields.
>>
>> This patch extends the KVMDevice struct with a new kda_addr_fixup
>> member. Its value is passed at registration time and OR'ed with the
>> resolved address on kvm_arm_set_device_addr().
>
>
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index 1e23640..4050017 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -34,6 +34,7 @@ int kvm_arm_vcpu_init(CPUState *cs);
>>   * @group: device control API group for setting addresses
>>   * @attr: device control API address type
>>   * @dev_fd: device control device file descriptor (or -1 if not supported)
>> + * @addr_fixup: value to be OR'ed with resolved address
>
> I think addr_fixup is a bit vague; how about
>  addr_fixed_bits ?
> (here and in the function argument name). Or something else if
> somebody has a better idea.

Thomas Huth suggested addr_ormask.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 6f467e6..eb9664e 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -558,7 +558,7 @@  static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
                             | KVM_VGIC_V2_ADDR_TYPE_DIST,
                             KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_V2_ADDR_TYPE_DIST,
-                            s->dev_fd);
+                            s->dev_fd, 0);
     /* CPU interface for current core. Unlike arm_gic, we don't
      * provide the "interface for core #N" memory regions, because
      * cores with a VGIC don't have those.
@@ -568,7 +568,7 @@  static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
                             | KVM_VGIC_V2_ADDR_TYPE_CPU,
                             KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_V2_ADDR_TYPE_CPU,
-                            s->dev_fd);
+                            s->dev_fd, 0);
 
     if (kvm_has_gsi_routing()) {
         /* set up irq routing */
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index eea6a73..271ebe4 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -103,7 +103,7 @@  static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
 
     /* register the base address */
     kvm_arm_register_device(&s->iomem_its_cntrl, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
-                            KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd);
+                            KVM_VGIC_ITS_ADDR_TYPE, s->dev_fd, 0);
 
     gicv3_its_init_mmio(s, NULL);
 
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index ec37177..93ac293 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -754,9 +754,9 @@  static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
                       KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true, &error_abort);
 
     kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
-                            KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd);
+                            KVM_VGIC_V3_ADDR_TYPE_DIST, s->dev_fd, 0);
     kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
-                            KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
+                            KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd, 0);
 
     if (kvm_has_gsi_routing()) {
         /* set up irq routing */
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 5141d0a..7eed8d4 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -184,10 +184,15 @@  unsigned long kvm_arch_vcpu_id(CPUState *cpu)
  * We use a MemoryListener to track mapping and unmapping of
  * the regions during board creation, so the board models don't
  * need to do anything special for the KVM case.
+ *
+ * Sometimes the address must be OR'ed with some other fields
+ * (for example for KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION).
+ * @kda_addr_fixup aims at storing the value of those fields.
  */
 typedef struct KVMDevice {
     struct kvm_arm_device_addr kda;
     struct kvm_device_attr kdattr;
+    uint64_t kda_addr_fixup;
     MemoryRegion *mr;
     QSLIST_ENTRY(KVMDevice) entries;
     int dev_fd;
@@ -234,6 +239,8 @@  static void kvm_arm_set_device_addr(KVMDevice *kd)
      */
     if (kd->dev_fd >= 0) {
         uint64_t addr = kd->kda.addr;
+
+        addr |= kd->kda_addr_fixup;
         attr->addr = (uintptr_t)&addr;
         ret = kvm_device_ioctl(kd->dev_fd, KVM_SET_DEVICE_ATTR, attr);
     } else {
@@ -266,7 +273,7 @@  static Notifier notify = {
 };
 
 void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
-                             uint64_t attr, int dev_fd)
+                             uint64_t attr, int dev_fd, uint64_t addr_fixup)
 {
     KVMDevice *kd;
 
@@ -286,6 +293,7 @@  void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
     kd->kdattr.group = group;
     kd->kdattr.attr = attr;
     kd->dev_fd = dev_fd;
+    kd->kda_addr_fixup = addr_fixup;
     QSLIST_INSERT_HEAD(&kvm_devices_head, kd, entries);
     memory_region_ref(kd->mr);
 }
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 1e23640..4050017 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -34,6 +34,7 @@  int kvm_arm_vcpu_init(CPUState *cs);
  * @group: device control API group for setting addresses
  * @attr: device control API address type
  * @dev_fd: device control device file descriptor (or -1 if not supported)
+ * @addr_fixup: value to be OR'ed with resolved address
  *
  * Remember the memory region @mr, and when it is mapped by the
  * machine model, tell the kernel that base address using the
@@ -45,7 +46,7 @@  int kvm_arm_vcpu_init(CPUState *cs);
  * address at the point where machine init is complete.
  */
 void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
-                             uint64_t attr, int dev_fd);
+                             uint64_t attr, int dev_fd, uint64_t addr_fixup);
 
 /**
  * kvm_arm_init_cpreg_list: