Re: [RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-19 Thread Christoffer Dall 
On Wed, Oct 17, 2012 at 4:29 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 14 October 2012 01:04, Christoffer Dall
 c.d...@virtualopensystems.com wrote:
 On ARM (and possibly other architectures) some bits are specific to the
 model being emulated for the guest and user space needs a way to tell
 the kernel about those bits.  An example is mmio device base addresses,
 where KVM must know the base address for a given device to properly
 emulate mmio accesses within a certain address range or directly map a
 device with virtualiation extensions into the guest address space.

 We try to make this API slightly more generic than for our specific use,
 but so far only the VGIC uses this feature.

 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 ---
  Documentation/virtual/kvm/api.txt |   30 ++
  arch/arm/include/asm/kvm.h|   13 +
  arch/arm/include/asm/kvm_mmu.h|1 +
  arch/arm/include/asm/kvm_vgic.h   |6 ++
  arch/arm/kvm/arm.c|   31 ++-
  arch/arm/kvm/vgic.c   |   34 +++---
  include/linux/kvm.h   |8 
  7 files changed, 119 insertions(+), 4 deletions(-)

 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 26e953d..30ddcac 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -2118,6 +2118,36 @@ for the emulated platofrm (see 
 KVM_SET_DEVICE_ADDRESS), but before the CPU is
  initally run.


 +4.80 KVM_SET_DEVICE_ADDRESS
 +
 +Capability: KVM_CAP_SET_DEVICE_ADDRESS
 +Architectures: arm
 +Type: vm ioctl
 +Parameters: struct kvm_device_address (in)
 +Returns: 0 on success, -1 on error
 +Errors:
 +  ENODEV: The device id is unknwown

 unknown

 +  ENXIO:  Device not supported in configuration

 in this configuration ? (I'm guessing this is for you tried to
 map a GIC when this CPU doesn't have a GIC and similar errors?)

 +  E2BIG:  Address outside of guest physical address space

 I would say outside rather than outside of here.

 +
 +struct kvm_device_address {
 +   __u32 id;
 +   __u64 addr;
 +};
 +
 +Specify a device address in the guest's physical address space where guests
 +can access emulated or directly exposed devices, which the host kernel needs
 +to know about. The id field is an architecture specific identifier for a
 +specific device.
 +
 +ARM divides the id field into two parts, a device ID and an address type id

 We should be consistent about whether ID is capitalised or not.


indeed

 +specific to the individual device.
 +
 +  bits:  | 31...16 | 15...0 |
 +  field: | device id   |  addr type id  |

 This doesn't say whether userspace is allowed to make this ioctl
 multiple times for the same device. This could be any of:
  * undefined behaviour
  * second call fails with some errno
  * second call overrides first one


I added an error condition EEXIST, but since this is trying to not be
arm-vgic specific this is really up to the individual device - maybe
we can have some polymorphic device that moves around later.

 It also doesn't say that you're supposed to call this after CREATE
 and before INIT of the irqchip. (Nor does it say what happens if
 you call it at some other time.)


same non-device specific argument as above.

Thanks,
-Christoffer
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-19 Thread Peter Maydell 
On 19 October 2012 19:46, Christoffer Dall
c.d...@virtualopensystems.com wrote:
 On Wed, Oct 17, 2012 at 4:29 PM, Peter Maydell peter.mayd...@linaro.org 
 wrote:
 This doesn't say whether userspace is allowed to make this ioctl
 multiple times for the same device. This could be any of:
  * undefined behaviour
  * second call fails with some errno
  * second call overrides first one


 I added an error condition EEXIST, but since this is trying to not be
 arm-vgic specific this is really up to the individual device - maybe
 we can have some polymorphic device that moves around later.

 It also doesn't say that you're supposed to call this after CREATE
 and before INIT of the irqchip. (Nor does it say what happens if
 you call it at some other time.)


 same non-device specific argument as above.

We could have a section in the docs that says On ARM platforms
there are devices X and Y and they have such-and-such properties
and requirements [and other devices later can have further docs
as appropriate].

-- PMM
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-19 Thread Christoffer Dall 
On Fri, Oct 19, 2012 at 4:24 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 19 October 2012 19:46, Christoffer Dall
 c.d...@virtualopensystems.com wrote:
 On Wed, Oct 17, 2012 at 4:29 PM, Peter Maydell peter.mayd...@linaro.org 
 wrote:
 This doesn't say whether userspace is allowed to make this ioctl
 multiple times for the same device. This could be any of:
  * undefined behaviour
  * second call fails with some errno
  * second call overrides first one


 I added an error condition EEXIST, but since this is trying to not be
 arm-vgic specific this is really up to the individual device - maybe
 we can have some polymorphic device that moves around later.

 It also doesn't say that you're supposed to call this after CREATE
 and before INIT of the irqchip. (Nor does it say what happens if
 you call it at some other time.)


 same non-device specific argument as above.

 We could have a section in the docs that says On ARM platforms
 there are devices X and Y and they have such-and-such properties
 and requirements [and other devices later can have further docs
 as appropriate].

sure, I can add that.

-Christoffer
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-19 Thread Christoffer Dall 
On Fri, Oct 19, 2012 at 4:27 PM, Christoffer Dall
c.d...@virtualopensystems.com wrote:
 On Fri, Oct 19, 2012 at 4:24 PM, Peter Maydell peter.mayd...@linaro.org 
 wrote:
 On 19 October 2012 19:46, Christoffer Dall
 c.d...@virtualopensystems.com wrote:
 On Wed, Oct 17, 2012 at 4:29 PM, Peter Maydell peter.mayd...@linaro.org 
 wrote:
 This doesn't say whether userspace is allowed to make this ioctl
 multiple times for the same device. This could be any of:
  * undefined behaviour
  * second call fails with some errno
  * second call overrides first one


 I added an error condition EEXIST, but since this is trying to not be
 arm-vgic specific this is really up to the individual device - maybe
 we can have some polymorphic device that moves around later.

 It also doesn't say that you're supposed to call this after CREATE
 and before INIT of the irqchip. (Nor does it say what happens if
 you call it at some other time.)


 same non-device specific argument as above.

 We could have a section in the docs that says On ARM platforms
 there are devices X and Y and they have such-and-such properties
 and requirements [and other devices later can have further docs
 as appropriate].



diff --git a/Documentation/virtual/kvm/api.txt
b/Documentation/virtual/kvm/api.txt
index 65aacc5..1380885 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2131,6 +2131,12 @@ specific to the individual device.
   bits:  | 31...16 | 15...0 |
   field: | device id   |  addr type id  |

+ARM currently only require this when using the in-kernel GIC support for the
+hardware vGIC features, using KVM_ARM_DEVICE_VGIC_V2 as the device id.  When
+setting the base address for the guest's mapping of the vGIC virtual CPU
+and distributor interface, the ioctl must be called after calling
+KVM_CREATE_IRQCHIP, but before calling KVM_RUN on any of the VCPUs.  Calling
+this ioctl twice for any of the base addresses will return -EEXIST.


 5. The kvm_run structure
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-17 Thread Peter Maydell 
On 14 October 2012 01:04, Christoffer Dall
c.d...@virtualopensystems.com wrote:
 On ARM (and possibly other architectures) some bits are specific to the
 model being emulated for the guest and user space needs a way to tell
 the kernel about those bits.  An example is mmio device base addresses,
 where KVM must know the base address for a given device to properly
 emulate mmio accesses within a certain address range or directly map a
 device with virtualiation extensions into the guest address space.

 We try to make this API slightly more generic than for our specific use,
 but so far only the VGIC uses this feature.

 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 ---
  Documentation/virtual/kvm/api.txt |   30 ++
  arch/arm/include/asm/kvm.h|   13 +
  arch/arm/include/asm/kvm_mmu.h|1 +
  arch/arm/include/asm/kvm_vgic.h   |6 ++
  arch/arm/kvm/arm.c|   31 ++-
  arch/arm/kvm/vgic.c   |   34 +++---
  include/linux/kvm.h   |8 
  7 files changed, 119 insertions(+), 4 deletions(-)

 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 26e953d..30ddcac 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -2118,6 +2118,36 @@ for the emulated platofrm (see 
 KVM_SET_DEVICE_ADDRESS), but before the CPU is
  initally run.


 +4.80 KVM_SET_DEVICE_ADDRESS
 +
 +Capability: KVM_CAP_SET_DEVICE_ADDRESS
 +Architectures: arm
 +Type: vm ioctl
 +Parameters: struct kvm_device_address (in)
 +Returns: 0 on success, -1 on error
 +Errors:
 +  ENODEV: The device id is unknwown

unknown

 +  ENXIO:  Device not supported in configuration

in this configuration ? (I'm guessing this is for you tried to
map a GIC when this CPU doesn't have a GIC and similar errors?)

 +  E2BIG:  Address outside of guest physical address space

I would say outside rather than outside of here.

 +
 +struct kvm_device_address {
 +   __u32 id;
 +   __u64 addr;
 +};
 +
 +Specify a device address in the guest's physical address space where guests
 +can access emulated or directly exposed devices, which the host kernel needs
 +to know about. The id field is an architecture specific identifier for a
 +specific device.
 +
 +ARM divides the id field into two parts, a device ID and an address type id

We should be consistent about whether ID is capitalised or not.

 +specific to the individual device.
 +
 +  bits:  | 31...16 | 15...0 |
 +  field: | device id   |  addr type id  |

This doesn't say whether userspace is allowed to make this ioctl
multiple times for the same device. This could be any of:
 * undefined behaviour
 * second call fails with some errno
 * second call overrides first one

It also doesn't say that you're supposed to call this after CREATE
and before INIT of the irqchip. (Nor does it say what happens if
you call it at some other time.)

-- PMM
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 2/3] KVM: ARM: Introduce KVM_SET_DEVICE_ADDRESS ioctl

2012-10-13 Thread Christoffer Dall 
On ARM (and possibly other architectures) some bits are specific to the
model being emulated for the guest and user space needs a way to tell
the kernel about those bits.  An example is mmio device base addresses,
where KVM must know the base address for a given device to properly
emulate mmio accesses within a certain address range or directly map a
device with virtualiation extensions into the guest address space.

We try to make this API slightly more generic than for our specific use,
but so far only the VGIC uses this feature.

Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
---
 Documentation/virtual/kvm/api.txt |   30 ++
 arch/arm/include/asm/kvm.h|   13 +
 arch/arm/include/asm/kvm_mmu.h|1 +
 arch/arm/include/asm/kvm_vgic.h   |6 ++
 arch/arm/kvm/arm.c|   31 ++-
 arch/arm/kvm/vgic.c   |   34 +++---
 include/linux/kvm.h   |8 
 7 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 26e953d..30ddcac 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2118,6 +2118,36 @@ for the emulated platofrm (see KVM_SET_DEVICE_ADDRESS), 
but before the CPU is
 initally run.
 
 
+4.80 KVM_SET_DEVICE_ADDRESS
+
+Capability: KVM_CAP_SET_DEVICE_ADDRESS
+Architectures: arm
+Type: vm ioctl
+Parameters: struct kvm_device_address (in)
+Returns: 0 on success, -1 on error
+Errors:
+  ENODEV: The device id is unknwown
+  ENXIO:  Device not supported in configuration
+  E2BIG:  Address outside of guest physical address space
+
+struct kvm_device_address {
+   __u32 id;
+   __u64 addr;
+};
+
+Specify a device address in the guest's physical address space where guests
+can access emulated or directly exposed devices, which the host kernel needs
+to know about. The id field is an architecture specific identifier for a
+specific device.
+
+ARM divides the id field into two parts, a device ID and an address type id
+specific to the individual device.
+
+  bits:  | 31...16 | 15...0 |
+  field: | device id   |  addr type id  |
+
+
+
 5. The kvm_run structure
 
 
diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h
index b6eaf0c..dfd60cc 100644
--- a/arch/arm/include/asm/kvm.h
+++ b/arch/arm/include/asm/kvm.h
@@ -42,6 +42,19 @@ struct kvm_regs {
 #define KVM_ARM_TARGET_CORTEX_A15  0
 #define KVM_ARM_NUM_TARGETS1
 
+/* KVM_SET_DEVICE_ADDRESS ioctl id encoding */
+#define KVM_DEVICE_TYPE_SHIFT  0
+#define KVM_DEVICE_TYPE_MASK   (0x  KVM_DEVICE_TYPE_SHIFT)
+#define KVM_DEVICE_ID_SHIFT16
+#define KVM_DEVICE_ID_MASK (0x  KVM_DEVICE_ID_SHIFT)
+
+/* Supported device IDs */
+#define KVM_ARM_DEVICE_VGIC_V2 0
+
+/* Supported VGIC address types  */
+#define KVM_VGIC_V2_ADDR_TYPE_DIST 0
+#define KVM_VGIC_V2_ADDR_TYPE_CPU  1
+
 struct kvm_vcpu_init {
__u32 target;
__u32 features[7];
diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
index ecfaaf0..0aef24f 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -26,6 +26,7 @@
  * To save a bit of memory and to avoid alignment issues we assume 39-bit IPA
  * for now, but remember that the level-1 table must be aligned to its size.
  */
+#define KVM_MAX_IPA((1ULL  38) - 1)
 #define PTRS_PER_PGD2  512
 #define PGD2_ORDER get_order(PTRS_PER_PGD2 * sizeof(pgd_t))
 
diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
index 588c637..a688132 100644
--- a/arch/arm/include/asm/kvm_vgic.h
+++ b/arch/arm/include/asm/kvm_vgic.h
@@ -242,6 +242,7 @@ struct kvm_exit_mmio;
 
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_set_addr(struct kvm *kvm, unsigned long type, u64 addr);
 int kvm_vgic_init(struct kvm *kvm);
 void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
 void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu);
@@ -261,6 +262,11 @@ static inline int kvm_vgic_hyp_init(void)
return 0;
 }
 
+static inline int kvm_vgic_set_addr(struct kvm *kvm, unsigned long type, u64 
addr)
+{
+   return 0;
+}
+
 static inline int kvm_vgic_init(struct kvm *kvm)
 {
return 0;
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 85c76e4..67c8cc2 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -207,6 +207,9 @@ int kvm_dev_ioctl_check_extension(long ext)
case KVM_CAP_COALESCED_MMIO:
r = KVM_COALESCED_MMIO_PAGE_OFFSET;
break;
+   case KVM_CAP_SET_DEVICE_ADDR:
+   r = 1;
+   break;
default:
r = 0;
break;
@@ -859,20 +862,46 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct 
kvm_dirty_log *log)
return