date:20150428

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

2015-04-28 Thread Christoffer Dall

On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote:
 On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote:
  Peter Maydell peter.mayd...@linaro.org writes:
  Does the kernel already have a conveniently implemented inject
  exception into guest lump of code? If so it might be less effort
  to do it that way round, maybe.
 
  So you pointed out we can't just re-inject the exceptions we get as we
  need to map from things like ESR_ELx_EC_WATCHPT_LOW to
  ESR_ELx_EC_WATCHPT_CUR before re-injection.
 
  Of course if it is as simple as modifying the ESR_EL1 register and
  returning +ve in the handle_exit path then I can do that but I assumed
  if any other wrangling needs doing it should be done in userspace.
 
 Well, somebody's got to do it, and it's the same amount of work
 either way (fiddling with ESR, making sure we direct the guest
 to the right exception vector entry point, maybe a few other
 things).
 
We already have code in the kernel to inject data/instruction aborts,
but not sure how much benefit there is in re-using that.  It's up to you
really, but I think the kernel code should be clear about what the
intention is so that we don't end up in a situation where: (1) The
intended behavior is unclear/vague, and (2) it doesn't actually work in
practice so nobody can follow the code.

Thanks,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

2015-04-28 Thread Christian Borntraeger

Am 28.04.2015 um 13:37 schrieb Paolo Bonzini:
 --- a/arch/powerpc/kvm/book3s_pr.c
 +++ b/arch/powerpc/kvm/book3s_pr.c
 @@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct 
 kvm_vcpu *vcpu,
  
  /* We get here with MSR.EE=1 */
  
 +local_irq_disable();
  trace_kvm_exit(exit_nr, vcpu);
 +local_irq_enable();
  kvm_guest_exit();
 
 This looks wrong.
 
Yes it is.

 --- a/include/linux/kvm_host.h
 +++ b/include/linux/kvm_host.h
 @@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void)
  
  static inline void kvm_guest_exit(void)
  {
 -unsigned long flags;
 -
 -local_irq_save(flags);
 
 Why no WARN_ON here?

Yes,WARN_ON makes sense.
Hmm, on the other hand the  irqs_disabled call might be somewhat expensive again
(would boil down on s390 to call stnsm (store and and system mask) once for 
query and once for setting.

so...
 
 I think the patches are sensible, especially since you can add warnings.
  This kind of code definitely knows if it has interrupts disabled or enabled
 
 Alternatively, the irq-disabled versions could be called
 __kvm_guest_{enter,exit}.  Then you can use those directly when it makes
 sense.

..having a special  __kvm_guest_{enter,exit} without the WARN_ON might be even
the cheapest way. In that way I could leave everything besides s390 alone and
arch maintainers can do a followup patch if appropriate.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

2015-04-28 Thread Paolo Bonzini



On 28/04/2015 16:10, Christian Borntraeger wrote:
  Alternatively, the irq-disabled versions could be called
  __kvm_guest_{enter,exit}.  Then you can use those directly when it makes
  sense.

 ..having a special  __kvm_guest_{enter,exit} without the WARN_ON might be even
 the cheapest way. In that way I could leave everything besides s390 alone and
 arch maintainers can do a followup patch if appropriate.

That's certainly fine with me.

Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

2015-04-28 Thread Alex Bennée


Christoffer Dall christoffer.d...@linaro.org writes:

 On Tue, Apr 28, 2015 at 10:34:12AM +0100, Peter Maydell wrote:
 On 28 April 2015 at 09:42, Alex Bennée alex.ben...@linaro.org wrote:
  Peter Maydell peter.mayd...@linaro.org writes:
  Does the kernel already have a conveniently implemented inject
  exception into guest lump of code? If so it might be less effort
  to do it that way round, maybe.
 
  So you pointed out we can't just re-inject the exceptions we get as we
  need to map from things like ESR_ELx_EC_WATCHPT_LOW to
  ESR_ELx_EC_WATCHPT_CUR before re-injection.
 
  Of course if it is as simple as modifying the ESR_EL1 register and
  returning +ve in the handle_exit path then I can do that but I assumed
  if any other wrangling needs doing it should be done in userspace.
 
 Well, somebody's got to do it, and it's the same amount of work
 either way (fiddling with ESR, making sure we direct the guest
 to the right exception vector entry point, maybe a few other
 things).
 
 We already have code in the kernel to inject data/instruction aborts,
 but not sure how much benefit there is in re-using that.  It's up to you
 really, but I think the kernel code should be clear about what the
 intention is so that we don't end up in a situation where: (1) The
 intended behavior is unclear/vague, and (2) it doesn't actually work in
 practice so nobody can follow the code.

Certainly there are some cases where the kernel doesn't have all the
information. For example it doesn't know if the soft break was inserted
by the guest or the host. That to me favours the let userspace deal
with the ugly approach.

-- 
Alex Bennée
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/3] qdev: check callback takes Object **target as third argument

2015-04-28 Thread Paolo Bonzini



On 28/04/2015 11:18, Eric Auger wrote:
 Check callback now takes as third argument an Object **. In
 object_set_link_property, we pass the property child as argument.
 We also assign the *child before the check call so that enhanced
 check can be performed in the callback. In case the check fails,
 the old value is restored and ref count is left unchanged.
 
 This typically makes possible to do checks both on the *child
 content (for instance a qemu_irq) and also perform some actions/
 checks on its container, which was not possible before.
 
 This is typically useful for starting irqfd setup in vfio platform
 use case.

s/typically/for example/

I can't say that starting irqfd setup in vfio platform is typical. :)

 diff --git a/include/qom/object.h b/include/qom/object.h
 index 4687fa1..0a7daff 100644
 --- a/include/qom/object.h
 +++ b/include/qom/object.h
 @@ -34,7 +34,7 @@ typedef struct InterfaceClass InterfaceClass;
  typedef struct InterfaceInfo InterfaceInfo;
  
  typedef void (*object_property_set_link_t)(Object *, const char *,
 -   Object *, Error **);
 +   Object **, Error **);

Let's make the new argument Object * const*, and rename the typedef to
LinkPropertySetter.

Ok with that change.

Paolo


  
  #define TYPE_OBJECT object
  
 @@ -1136,7 +1136,7 @@ typedef enum {
   * an error.
   */
  void object_property_allow_set_link(Object *, const char *,
 -Object *, Error **);
 +Object **, Error **);
  
  /**
   * object_property_add_link:
 @@ -1168,8 +1168,7 @@ void object_property_allow_set_link(Object *, const 
 char *,
   */
  void object_property_add_link(Object *obj, const char *name,
const char *type, Object **child,
 -  void (*check)(Object *obj, const char *name,
 -Object *val, Error **errp),
 +  object_property_set_link_t check,
ObjectPropertyLinkFlags flags,
Error **errp);
  
 diff --git a/qom/object.c b/qom/object.c
 index b8dff43..cc9ed87 100644
 --- a/qom/object.c
 +++ b/qom/object.c
 @@ -1112,14 +1112,14 @@ out:
  }
  
  void object_property_allow_set_link(Object *obj, const char *name,
 -Object *val, Error **errp)
 +Object **target, Error **errp)
  {
  /* Allow the link to be set, always */
  }
  
  typedef struct {
  Object **child;
 -void (*check)(Object *, const char *, Object *, Error **);
 +void (*check)(Object *, const char *, Object **, Error **);
  ObjectPropertyLinkFlags flags;
  } LinkProperty;
  
 @@ -1201,14 +1201,17 @@ static void object_set_link_property(Object *obj, 
 Visitor *v, void *opaque,
  return;
  }
  
 -prop-check(obj, name, new_target, local_err);
 +object_ref(new_target);
 +*child = new_target;
 +
 +prop-check(obj, name, child, local_err);
  if (local_err) {
  error_propagate(errp, local_err);
 +*child = old_target;
 +object_ref(new_target);
  return;
  }
  
 -object_ref(new_target);
 -*child = new_target;
  object_unref(old_target);
  }
  
 @@ -1233,7 +1236,7 @@ static void object_release_link_property(Object *obj, 
 const char *name,
  void object_property_add_link(Object *obj, const char *name,
const char *type, Object **child,
void (*check)(Object *, const char *,
 -Object *, Error **),
 +Object **, Error **),
ObjectPropertyLinkFlags flags,
Error **errp)
  {
 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 01/12] linux-headers: update headers according to 4.1-rc0

2015-04-28 Thread Eric Auger

This includes, among other things, VFIO platform driver and
irqfd/arm.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v12 - v13:
- update for 4.1-rc0 headers

v10 - v11:
- only includes header modifications related to vfio platform
  driver v14 and not those related to
  vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1
---
 include/standard-headers/linux/virtio_blk.h |   8 +-
 linux-headers/asm-arm/kvm.h |   3 +
 linux-headers/asm-arm64/kvm.h   |   3 +
 linux-headers/asm-mips/kvm.h| 164 +---
 linux-headers/asm-s390/kvm.h|   4 +
 linux-headers/linux/kvm.h   |  65 ++-
 linux-headers/linux/vfio.h  |   2 +
 7 files changed, 183 insertions(+), 66 deletions(-)

diff --git a/include/standard-headers/linux/virtio_blk.h 
b/include/standard-headers/linux/virtio_blk.h
index 12016b4..cd601f4 100644
--- a/include/standard-headers/linux/virtio_blk.h
+++ b/include/standard-headers/linux/virtio_blk.h
@@ -58,7 +58,7 @@ struct virtio_blk_config {
uint32_t size_max;
/* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
uint32_t seg_max;
-   /* geometry the device (if VIRTIO_BLK_F_GEOMETRY) */
+   /* geometry of the device (if VIRTIO_BLK_F_GEOMETRY) */
struct virtio_blk_geometry {
uint16_t cylinders;
uint8_t heads;
@@ -117,7 +117,11 @@ struct virtio_blk_config {
 #define VIRTIO_BLK_T_BARRIER   0x8000
 #endif /* !VIRTIO_BLK_NO_LEGACY */
 
-/* This is the first element of the read scatter-gather list. */
+/*
+ * This comes first in the read scatter-gather list.
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated,
+ * this is the first element of the read scatter-gather list.
+ */
 struct virtio_blk_outhdr {
/* VIRTIO_BLK_T* */
__virtio32 type;
diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
index 0db25bc..2499867 100644
--- a/linux-headers/asm-arm/kvm.h
+++ b/linux-headers/asm-arm/kvm.h
@@ -198,6 +198,9 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX127
 
+/* One single KVM irqchip, ie. the VGIC */
+#define KVM_NR_IRQCHIPS  1
+
 /* PSCI interface */
 #define KVM_PSCI_FN_BASE   0x95c1ba5e
 #define KVM_PSCI_FN(n) (KVM_PSCI_FN_BASE + (n))
diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 3ef77a4..c154c0b 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -191,6 +191,9 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX127
 
+/* One single KVM irqchip, ie. the VGIC */
+#define KVM_NR_IRQCHIPS  1
+
 /* PSCI interface */
 #define KVM_PSCI_FN_BASE   0x95c1ba5e
 #define KVM_PSCI_FN(n) (KVM_PSCI_FN_BASE + (n))
diff --git a/linux-headers/asm-mips/kvm.h b/linux-headers/asm-mips/kvm.h
index 2c04b6d..6985eb5 100644
--- a/linux-headers/asm-mips/kvm.h
+++ b/linux-headers/asm-mips/kvm.h
@@ -36,77 +36,85 @@ struct kvm_regs {
 
 /*
  * for KVM_GET_FPU and KVM_SET_FPU
- *
- * If Status[FR] is zero (32-bit FPU), the upper 32-bits of the FPRs
- * are zero filled.
  */
 struct kvm_fpu {
-   __u64 fpr[32];
-   __u32 fir;
-   __u32 fccr;
-   __u32 fexr;
-   __u32 fenr;
-   __u32 fcsr;
-   __u32 pad;
 };
 
 
 /*
- * For MIPS, we use KVM_SET_ONE_REG and KVM_GET_ONE_REG to access CP0
+ * For MIPS, we use KVM_SET_ONE_REG and KVM_GET_ONE_REG to access various
  * registers.  The id field is broken down as follows:
  *
- *  bits[2..0]   - Register 'sel' index.
- *  bits[7..3]   - Register 'rd'  index.
- *  bits[15..8]  - Must be zero.
- *  bits[31..16] - 1 - CP0 registers.
- *  bits[51..32] - Must be zero.
  *  bits[63..52] - As per linux/kvm.h
+ *  bits[51..32] - Must be zero.
+ *  bits[31..16] - Register set.
+ *
+ * Register set = 0: GP registers from kvm_regs (see definitions below).
+ *
+ * Register set = 1: CP0 registers.
+ *  bits[15..8]  - Must be zero.
+ *  bits[7..3]   - Register 'rd'  index.
+ *  bits[2..0]   - Register 'sel' index.
+ *
+ * Register set = 2: KVM specific registers (see definitions below).
+ *
+ * Register set = 3: FPU / MSA registers (see definitions below).
  *
  * Other sets registers may be added in the future.  Each set would
  * have its own identifier in bits[31..16].
- *
- * The registers defined in struct kvm_regs are also accessible, the
- * id values for these are below.
  */
 
-#define KVM_REG_MIPS_R0 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 0)
-#define KVM_REG_MIPS_R1 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 1)
-#define KVM_REG_MIPS_R2 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 2)
-#define KVM_REG_MIPS_R3 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 3)
-#define KVM_REG_MIPS_R4 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 4)
-#define KVM_REG_MIPS_R5 (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 5)

[PATCH v13 10/12] qdev: check callback takes the property child as third argument

2015-04-28 Thread Eric Auger

Check callback now takes as third argument an Object * const*. In
object_set_link_property, we pass the property child as argument.
We also assign the *child before the check call so that enhanced
check can be performed in the callback. In case the check fails,
the old value is restored and ref count is left unchanged.

This typically makes possible to do checks both on the *child
content (for instance a qemu_irq) and also perform some actions/
checks on its container, which was not possible before.

This is for example useful for starting irqfd setup in vfio platform
use case.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v1 - v2:
- Object ** becomes Object * const*
- rename patch title
---
 hw/core/qdev-properties.c|  3 ++-
 include/hw/qdev-properties.h |  3 ++-
 include/qom/object.h |  4 ++--
 qom/object.c | 16 +---
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 570d5f0..7e00cd2 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -22,7 +22,8 @@ void qdev_prop_set_after_realize(DeviceState *dev, const char 
*name,
 }
 
 void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
- Object *val, Error **errp)
+ Object * const *target,
+ Error **errp)
 {
 DeviceState *dev = DEVICE(obj);
 
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index d67dad5..67ac457 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -213,6 +213,7 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
char *name,
  * object_property_add_link().
  */
 void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
- Object *val, Error **errp);
+ Object * const *target,
+ Error **errp);
 
 #endif
diff --git a/include/qom/object.h b/include/qom/object.h
index 95d1a1d..ab60c16 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -34,7 +34,7 @@ typedef struct InterfaceClass InterfaceClass;
 typedef struct InterfaceInfo InterfaceInfo;
 
 typedef void (*LinkPropertySetter)(Object *, const char *,
-   Object *, Error **);
+  Object * const *, Error **);
 
 #define TYPE_OBJECT object
 
@@ -1136,7 +1136,7 @@ typedef enum {
  * an error.
  */
 void object_property_allow_set_link(Object *, const char *,
-Object *, Error **);
+Object * const *, Error **);
 
 /**
  * object_property_add_link:
diff --git a/qom/object.c b/qom/object.c
index b8dff43..ed286e7 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1112,14 +1112,14 @@ out:
 }
 
 void object_property_allow_set_link(Object *obj, const char *name,
-Object *val, Error **errp)
+Object * const *target, Error **errp)
 {
 /* Allow the link to be set, always */
 }
 
 typedef struct {
 Object **child;
-void (*check)(Object *, const char *, Object *, Error **);
+LinkPropertySetter check;
 ObjectPropertyLinkFlags flags;
 } LinkProperty;
 
@@ -1201,14 +1201,17 @@ static void object_set_link_property(Object *obj, 
Visitor *v, void *opaque,
 return;
 }
 
-prop-check(obj, name, new_target, local_err);
+object_ref(new_target);
+*child = new_target;
+
+prop-check(obj, name, child, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
+*child = old_target;
+object_ref(new_target);
 return;
 }
 
-object_ref(new_target);
-*child = new_target;
 object_unref(old_target);
 }
 
@@ -1232,8 +1235,7 @@ static void object_release_link_property(Object *obj, 
const char *name,
 
 void object_property_add_link(Object *obj, const char *name,
   const char *type, Object **child,
-  void (*check)(Object *, const char *,
-Object *, Error **),
+  LinkPropertySetter check,
   ObjectPropertyLinkFlags flags,
   Error **errp)
 {
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 11/12] sysbus: add irq_set_hook

2015-04-28 Thread Eric Auger

Add a new callback in the SysBusDeviceClass. This callback now can
be overriden by devices inheriting from sysbus. By default the callback
is set to the dummy object_property_allow_set_link callback.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v1 - v2:
- use new LinkPropertySetter type
---
 hw/core/sysbus.c| 7 ++-
 include/hw/sysbus.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 1bcb64d..ee573b4 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -159,8 +159,10 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, 
hwaddr addr,
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
+SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(dev);
+
 qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1,
- object_property_allow_set_link);
+ sbc-irq_set_hook);
 }
 
 /* Pass IRQs from a target device.  */
@@ -311,8 +313,11 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
 static void sysbus_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
+SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
+
 k-init = sysbus_device_init;
 k-bus_type = TYPE_SYSTEM_BUS;
+sbc-irq_set_hook = object_property_allow_set_link;
 }
 
 static const TypeInfo sysbus_device_type_info = {
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index d1f3f00..b8dde79 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -41,6 +41,7 @@ typedef struct SysBusDeviceClass {
 /* public */
 
 int (*init)(SysBusDevice *dev);
+LinkPropertySetter irq_set_hook;
 } SysBusDeviceClass;
 
 struct SysBusDevice {
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 04/12] hw/vfio/platform: calxeda xgmac device

2015-04-28 Thread Eric Auger

The platform device class has become abstract. This patch introduces
a calxeda xgmac device that derives from it.

Signed-off-by: Eric Auger eric.au...@linaro.org
Reviewed-by: Alex Benneealex.ben...@linaro.org

---
v10 - v11:
- add Alex Reviewed-by
- move virt modifications in a separate patch

v8 - v9:
- renamed calxeda_xgmac.c into calxeda-xgmac.c

v7 - v8:
- add a comment in the header about the MMIO regions and IRQ which
  are exposed by the device

v5 - v6
- back again following Alex Graf advises
- fix a bug related to compat override

v4 - v5:
removed since device tree was moved to hw/arm/dyn_sysbus_devtree.c

v4: creation for device tree specialization

Conflicts:
hw/arm/virt.c
---
 hw/vfio/Makefile.objs|  1 +
 hw/vfio/calxeda-xgmac.c  | 54 
 include/hw/vfio/vfio-calxeda-xgmac.h | 46 ++
 3 files changed, 101 insertions(+)
 create mode 100644 hw/vfio/calxeda-xgmac.c
 create mode 100644 include/hw/vfio/vfio-calxeda-xgmac.h

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index c5c76fe..d540c9d 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -2,4 +2,5 @@ ifeq ($(CONFIG_LINUX), y)
 obj-$(CONFIG_SOFTMMU) += common.o
 obj-$(CONFIG_PCI) += pci.o
 obj-$(CONFIG_SOFTMMU) += platform.o
+obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
 endif
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
new file mode 100644
index 000..c4b8fef
--- /dev/null
+++ b/hw/vfio/calxeda-xgmac.c
@@ -0,0 +1,54 @@
+/*
+ * calxeda xgmac VFIO device
+ *
+ * Copyright Linaro Limited, 2014
+ *
+ * Authors:
+ *  Eric Auger eric.au...@linaro.org
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include hw/vfio/vfio-calxeda-xgmac.h
+
+static void calxeda_xgmac_realize(DeviceState *dev, Error **errp)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
+VFIOCalxedaXgmacDeviceClass *k = VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(dev);
+
+vdev-compat = g_strdup(calxeda,hb-xgmac);
+
+k-parent_realize(dev, errp);
+}
+
+static const VMStateDescription vfio_platform_vmstate = {
+.name = TYPE_VFIO_CALXEDA_XGMAC,
+.unmigratable = 1,
+};
+
+static void vfio_calxeda_xgmac_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VFIOCalxedaXgmacDeviceClass *vcxc =
+VFIO_CALXEDA_XGMAC_DEVICE_CLASS(klass);
+vcxc-parent_realize = dc-realize;
+dc-realize = calxeda_xgmac_realize;
+dc-desc = VFIO Calxeda XGMAC;
+}
+
+static const TypeInfo vfio_calxeda_xgmac_dev_info = {
+.name = TYPE_VFIO_CALXEDA_XGMAC,
+.parent = TYPE_VFIO_PLATFORM,
+.instance_size = sizeof(VFIOCalxedaXgmacDevice),
+.class_init = vfio_calxeda_xgmac_class_init,
+.class_size = sizeof(VFIOCalxedaXgmacDeviceClass),
+};
+
+static void register_calxeda_xgmac_dev_type(void)
+{
+type_register_static(vfio_calxeda_xgmac_dev_info);
+}
+
+type_init(register_calxeda_xgmac_dev_type)
diff --git a/include/hw/vfio/vfio-calxeda-xgmac.h 
b/include/hw/vfio/vfio-calxeda-xgmac.h
new file mode 100644
index 000..f994775
--- /dev/null
+++ b/include/hw/vfio/vfio-calxeda-xgmac.h
@@ -0,0 +1,46 @@
+/*
+ * VFIO calxeda xgmac device
+ *
+ * Copyright Linaro Limited, 2014
+ *
+ * Authors:
+ *  Eric Auger eric.au...@linaro.org
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef HW_VFIO_VFIO_CALXEDA_XGMAC_H
+#define HW_VFIO_VFIO_CALXEDA_XGMAC_H
+
+#include hw/vfio/vfio-platform.h
+
+#define TYPE_VFIO_CALXEDA_XGMAC vfio-calxeda-xgmac
+
+/**
+ * This device exposes:
+ * - a single MMIO region corresponding to its register space
+ * - 3 IRQS (main and 2 power related IRQs)
+ */
+typedef struct VFIOCalxedaXgmacDevice {
+VFIOPlatformDevice vdev;
+} VFIOCalxedaXgmacDevice;
+
+typedef struct VFIOCalxedaXgmacDeviceClass {
+/* private */
+VFIOPlatformDeviceClass parent_class;
+/* public */
+DeviceRealize parent_realize;
+} VFIOCalxedaXgmacDeviceClass;
+
+#define VFIO_CALXEDA_XGMAC_DEVICE(obj) \
+ OBJECT_CHECK(VFIOCalxedaXgmacDevice, (obj), TYPE_VFIO_CALXEDA_XGMAC)
+#define VFIO_CALXEDA_XGMAC_DEVICE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(VFIOCalxedaXgmacDeviceClass, (klass), \
+TYPE_VFIO_CALXEDA_XGMAC)
+#define VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(VFIOCalxedaXgmacDeviceClass, (obj), \
+  TYPE_VFIO_CALXEDA_XGMAC)
+
+#endif
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 05/12] hw/arm/sysbus-fdt: enable vfio-calxeda-xgmac dynamic instantiation

2015-04-28 Thread Eric Auger

This patch allows the instantiation of the vfio-calxeda-xgmac device
from the QEMU command line (-device vfio-calxeda-xgmac,host=device).

A specialized device tree node is created for the guest, containing
compat, dma-coherent, reg and interrupts properties.

Signed-off-by: Eric Auger eric.au...@linaro.org

---
v12 - v13:
- use platform_bus_get_mmio_addr instead of deprecated mmio[0] property.
  Thanks to Bharat who pointed this issue out.
- use cpu_to_be32 to mmio_base  size (Vikram report)

v10 - v11:
- add dma-coherent property to calxeda midway xgmac node (fix)
- use qemu_fdt_setprop to add reg property instead of
  qemu_fdt_setprop_sized_cells_from_array
- commit message rewording

v8 - v9:
- properly free resources in case of errors in
  add_calxeda_midway_xgmac_fdt_node

v7 - v8:
- move the add_fdt_node_functions array declaration between the device
  specific code and the generic code to avoid forward declarations of
  decice specific functions
- rename add_basic_vfio_fdt_node into
  add_calxeda_midway_xgmac_fdt_node

v6 - v7:
- compat string re-formatting removed since compat string is not exposed
  anymore as a user option
- VFIO IRQ kick-off removed from sysbus-fdt and moved to VFIO platform
  device
---
 hw/arm/sysbus-fdt.c | 72 +
 1 file changed, 72 insertions(+)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 3038b94..3d67acf 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -26,6 +26,8 @@
 #include sysemu/device_tree.h
 #include hw/platform-bus.h
 #include sysemu/sysemu.h
+#include hw/vfio/vfio-platform.h
+#include hw/vfio/vfio-calxeda-xgmac.h
 
 /*
  * internal struct that contains the information to create dynamic
@@ -53,11 +55,81 @@ typedef struct NodeCreationPair {
 int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
 } NodeCreationPair;
 
+/* Device Specific Code */
+
+/**
+ * add_calxeda_midway_xgmac_fdt_node
+ *
+ * Generates a simple node with following properties:
+ * compatible string, regs, interrupts, dma-coherent
+ */
+static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+PlatformBusFDTData *data = opaque;
+PlatformBusDevice *pbus = data-pbus;
+void *fdt = data-fdt;
+const char *parent_node = data-pbus_node_name;
+int compat_str_len, i, ret = -1;
+char *nodename;
+uint32_t *irq_attr, *reg_attr;
+uint64_t mmio_base, irq_number;
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+VFIODevice *vbasedev = vdev-vbasedev;
+
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+nodename = g_strdup_printf(%s/%s@% PRIx64, parent_node,
+   vbasedev-name, mmio_base);
+qemu_fdt_add_subnode(fdt, nodename);
+
+compat_str_len = strlen(vdev-compat) + 1;
+qemu_fdt_setprop(fdt, nodename, compatible,
+  vdev-compat, compat_str_len);
+
+qemu_fdt_setprop(fdt, nodename, dma-coherent, , 0);
+
+reg_attr = g_new(uint32_t, vbasedev-num_regions*2);
+for (i = 0; i  vbasedev-num_regions; i++) {
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
+reg_attr[2*i] = cpu_to_be32(mmio_base);
+reg_attr[2*i+1] = cpu_to_be32(
+memory_region_size(vdev-regions[i]-mem));
+}
+ret = qemu_fdt_setprop(fdt, nodename, reg, reg_attr,
+   vbasedev-num_regions*2*sizeof(uint32_t));
+if (ret) {
+error_report(could not set reg property of node %s, nodename);
+goto fail_reg;
+}
+
+irq_attr = g_new(uint32_t, vbasedev-num_irqs*3);
+for (i = 0; i  vbasedev-num_irqs; i++) {
+irq_number = platform_bus_get_irqn(pbus, sbdev , i)
+ + data-irq_start;
+irq_attr[3*i] = cpu_to_be32(0);
+irq_attr[3*i+1] = cpu_to_be32(irq_number);
+irq_attr[3*i+2] = cpu_to_be32(0x4);
+}
+   ret = qemu_fdt_setprop(fdt, nodename, interrupts,
+ irq_attr, vbasedev-num_irqs*3*sizeof(uint32_t));
+if (ret) {
+error_report(could not set interrupts property of node %s,
+ nodename);
+}
+g_free(irq_attr);
+fail_reg:
+g_free(reg_attr);
+g_free(nodename);
+return ret;
+}
+
 /* list of supported dynamic sysbus devices */
 static const NodeCreationPair add_fdt_node_functions[] = {
+{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
 {, NULL}, /* last element */
 };
 
+/* Generic Code */
+
 /**
  * add_fdt_node - add the device tree node of a dynamic sysbus device
  *
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 07/12] kvm-all.c: add qemu_irq/gsi hash table and utility routines

2015-04-28 Thread Eric Auger

VFIO platform device needs to setup irqfd but it does not know the
gsi corresponding to the device qemu_irq. This series proposes to
store a hash table in kvm_state using the qemu_irq as key and the gsi
as a value.

kvm_irqchip_set_qemuirq_gsi allows to insert such a pair. The interrupt
controller is supposed to use it.

kvm_irqchip_[add, remove]_irqfd_notifier allows to setup/tear down
irqfd directly from the qemu_irq.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v2 - v3:
- rename kvm_irqchip_[add, remove]_qemuirq_irqfd_notifier into
  kvm_irqchip_[add, remove]_irqfd_notifier. Possible since legacy
  functions were also renamed with _gsi suffix.

V1 - v2:
- qemu_irq get_gsi callback replaced by hash table stored in kvm
---
 include/sysemu/kvm.h |  6 ++
 kvm-all.c| 35 +++
 2 files changed, 41 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0f28d6f..bc3f230 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -18,6 +18,7 @@
 #include config-host.h
 #include qemu/queue.h
 #include qom/cpu.h
+#include hw/irq.h
 
 #ifdef CONFIG_KVM
 #include linux/kvm.h
@@ -417,6 +418,11 @@ int kvm_irqchip_add_irqfd_notifier_gsi(KVMState *s, 
EventNotifier *n,
EventNotifier *rn, int virq);
 int kvm_irqchip_remove_irqfd_notifier_gsi(KVMState *s, EventNotifier *n,
   int virq);
+int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n,
+   EventNotifier *rn, qemu_irq irq);
+int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n,
+  qemu_irq irq);
+void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi);
 void kvm_pc_gsi_handler(void *opaque, int n, int level);
 void kvm_pc_setup_irq_routing(bool pci_enabled);
 void kvm_init_irq_routing(KVMState *s);
diff --git a/kvm-all.c b/kvm-all.c
index 42bb923..6531062 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -37,6 +37,7 @@
 #include exec/address-spaces.h
 #include qemu/event_notifier.h
 #include trace.h
+#include hw/irq.h
 
 #include hw/boards.h
 
@@ -99,6 +100,7 @@ struct KVMState
  * unsigned, and treating them as signed here can break things */
 unsigned irq_set_ioctl;
 unsigned int sigmask_len;
+GHashTable *gsimap;
 #ifdef KVM_CAP_IRQ_ROUTING
 struct kvm_irq_routing *irq_routes;
 int nr_allocated_irq_routes;
@@ -1382,6 +1384,37 @@ int kvm_irqchip_remove_irqfd_notifier_gsi(KVMState *s, 
EventNotifier *n,
false);
 }
 
+int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n,
+   EventNotifier *rn, qemu_irq irq)
+{
+gpointer key, gsi;
+gboolean found = g_hash_table_lookup_extended(s-gsimap, irq, key, gsi);
+
+if (!found) {
+return -ENXIO;
+} else {
+return kvm_irqchip_add_irqfd_notifier_gsi(s, n, rn, 
GPOINTER_TO_UINT(gsi));
+}
+}
+
+int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n,
+  qemu_irq irq)
+{
+gpointer key, gsi;
+gboolean found = g_hash_table_lookup_extended(s-gsimap, irq, key, gsi);
+
+if (!found) {
+return -ENXIO;
+} else {
+return kvm_irqchip_remove_irqfd_notifier_gsi(s, n, 
GPOINTER_TO_INT(gsi));
+}
+}
+
+void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi)
+{
+g_hash_table_insert(s-gsimap, irq, GINT_TO_POINTER(gsi));
+}
+
 static int kvm_irqchip_create(MachineState *machine, KVMState *s)
 {
 int ret;
@@ -1414,6 +1447,8 @@ static int kvm_irqchip_create(MachineState *machine, 
KVMState *s)
 
 kvm_init_irq_routing(s);
 
+s-gsimap = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 return 0;
 }
 
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 09/12] qdev: pass the check callback to qdev_init_gpio_out_named

2015-04-28 Thread Eric Auger

qdev_init_gpio_out_named takes a new argument corresponding to the
check callback passed to object_property_add_link. In qdev_init_gpio_out
and sysbus_init_irq, this callback is currently set to the dummy
object_property_allow_set_link.

This will allow qdev_init_gpio_out_named callers to specialize this
callback. A subsequent patch will implement that for sysbus.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v1 - v2:
- fix qdev_init_gpio_out_named call in sysbus_init_irq
- rewording of commit message
---
 hw/core/qdev.c | 8 +---
 hw/core/sysbus.c   | 3 ++-
 include/hw/qdev-core.h | 3 ++-
 include/qom/object.h   | 6 --
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..857217d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -452,7 +452,8 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler 
handler, int n)
 }
 
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
-  const char *name, int n)
+  const char *name, int n,
+  LinkPropertySetter check_cb)
 {
 int i;
 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
@@ -465,7 +466,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq 
*pins,
 memset(pins[i], 0, sizeof(*pins));
 object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
  (Object **)pins[i],
- object_property_allow_set_link,
+ check_cb,
  OBJ_PROP_LINK_UNREF_ON_RELEASE,
  error_abort);
 }
@@ -474,7 +475,8 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq 
*pins,
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
 {
-qdev_init_gpio_out_named(dev, pins, NULL, n);
+qdev_init_gpio_out_named(dev, pins, NULL, n,
+ object_property_allow_set_link);
 }
 
 qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index b53c351..1bcb64d 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -159,7 +159,8 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, 
hwaddr addr,
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
+qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1,
+ object_property_allow_set_link);
 }
 
 /* Pass IRQs from a target device.  */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4e673f9..0885f39 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -293,7 +293,8 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, 
int n);
 void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
  const char *name, int n);
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
-  const char *name, int n);
+  const char *name, int n,
+  LinkPropertySetter fn);
 
 void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
  const char *name);
diff --git a/include/qom/object.h b/include/qom/object.h
index d2d7748..95d1a1d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -33,6 +33,9 @@ typedef struct TypeInfo TypeInfo;
 typedef struct InterfaceClass InterfaceClass;
 typedef struct InterfaceInfo InterfaceInfo;
 
+typedef void (*LinkPropertySetter)(Object *, const char *,
+   Object *, Error **);
+
 #define TYPE_OBJECT object
 
 /**
@@ -1165,8 +1168,7 @@ void object_property_allow_set_link(Object *, const char 
*,
  */
 void object_property_add_link(Object *obj, const char *name,
   const char *type, Object **child,
-  void (*check)(Object *obj, const char *name,
-Object *val, Error **errp),
+  LinkPropertySetter check,
   ObjectPropertyLinkFlags flags,
   Error **errp);
 
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v13 03/12] hw/vfio/platform: add irq assignment

2015-04-28 Thread Eric Auger

This patch adds the code requested to assign interrupts to
a guest. The interrupts are mediated through user handled
eventfds only.

Signed-off-by: Eric Auger eric.au...@linaro.org

---
v12 - v13:
- start user-side eventfd handling at realize time
- remove start_irq_fn

v10 - v11:
- use block declaration when possible
- change order of vfio_platform_eoi vs vfio_intp_interrupt
- introduce vfio_intp_inject_pending_lockheld following Alex Bennee
  comments
- remove unmasking/masked when setting up VFIO signaling
- remove unused kvm_accel member in VFIOINTp struct
- add flags member in VFIOINTp in order to properly discriminate
  edge/level-sensitive IRQs; unmask the physical IRQ only in case of
  level-sensitive IRQ
- some comment rewording

v8 - v9:
- free irq related resources in case of error in vfio_populate_device
---
 hw/vfio/platform.c  | 331 +++-
 include/hw/vfio/vfio-platform.h |  32 
 trace-events|   7 +
 3 files changed, 369 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 48dfc55..f484764 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -22,10 +22,299 @@
 #include qemu/range.h
 #include sysemu/sysemu.h
 #include exec/memory.h
+#include qemu/queue.h
 #include hw/sysbus.h
 #include trace.h
 #include hw/platform-bus.h
 
+/*
+ * Functions used whatever the injection method
+ */
+
+/**
+ * vfio_init_intp - allocate, initialize the IRQ struct pointer
+ * and add it into the list of IRQs
+ * @vbasedev: the VFIO device handle
+ * @info: irq info struct retrieved from VFIO driver
+ */
+static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
+struct vfio_irq_info info)
+{
+int ret;
+VFIOPlatformDevice *vdev =
+container_of(vbasedev, VFIOPlatformDevice, vbasedev);
+SysBusDevice *sbdev = SYS_BUS_DEVICE(vdev);
+VFIOINTp *intp;
+
+intp = g_malloc0(sizeof(*intp));
+intp-vdev = vdev;
+intp-pin = info.index;
+intp-flags = info.flags;
+intp-state = VFIO_IRQ_INACTIVE;
+
+sysbus_init_irq(sbdev, intp-qemuirq);
+
+/* Get an eventfd for trigger */
+ret = event_notifier_init(intp-interrupt, 0);
+if (ret) {
+g_free(intp);
+error_report(vfio: Error: trigger event_notifier_init failed );
+return NULL;
+}
+
+QLIST_INSERT_HEAD(vdev-intp_list, intp, next);
+return intp;
+}
+
+/**
+ * vfio_set_trigger_eventfd - set VFIO eventfd handling
+ *
+ * @intp: IRQ struct handle
+ * @handler: handler to be called on eventfd signaling
+ *
+ * Setup VFIO signaling and attach an optional user-side handler
+ * to the eventfd
+ */
+static int vfio_set_trigger_eventfd(VFIOINTp *intp,
+eventfd_user_side_handler_t handler)
+{
+VFIODevice *vbasedev = intp-vdev-vbasedev;
+struct vfio_irq_set *irq_set;
+int argsz, ret;
+int32_t *pfd;
+
+argsz = sizeof(*irq_set) + sizeof(*pfd);
+irq_set = g_malloc0(argsz);
+irq_set-argsz = argsz;
+irq_set-flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER;
+irq_set-index = intp-pin;
+irq_set-start = 0;
+irq_set-count = 1;
+pfd = (int32_t *)irq_set-data;
+*pfd = event_notifier_get_fd(intp-interrupt);
+qemu_set_fd_handler(*pfd, (IOHandler *)handler, NULL, intp);
+ret = ioctl(vbasedev-fd, VFIO_DEVICE_SET_IRQS, irq_set);
+g_free(irq_set);
+if (ret  0) {
+error_report(vfio: Failed to set trigger eventfd: %m);
+qemu_set_fd_handler(*pfd, NULL, NULL, NULL);
+}
+return ret;
+}
+
+/*
+ * Functions only used when eventfds are handled on user-side
+ * ie. without irqfd
+ */
+
+/**
+ * vfio_mmap_set_enabled - enable/disable the fast path mode
+ * @vdev: the VFIO platform device
+ * @enabled: the target mmap state
+ *
+ * enabled = true ~ fast path = MMIO region is mmaped (no KVM TRAP);
+ * enabled = false ~ slow path = MMIO region is trapped and region callbacks
+ * are called; slow path enables to trap the device IRQ status register reset
+*/
+
+static void vfio_mmap_set_enabled(VFIOPlatformDevice *vdev, bool enabled)
+{
+int i;
+
+trace_vfio_platform_mmap_set_enabled(enabled);
+
+for (i = 0; i  vdev-vbasedev.num_regions; i++) {
+VFIORegion *region = vdev-regions[i];
+
+memory_region_set_enabled(region-mmap_mem, enabled);
+}
+}
+
+/**
+ * vfio_intp_mmap_enable - timer function, restores the fast path
+ * if there is no more active IRQ
+ * @opaque: actually points to the VFIO platform device
+ *
+ * Called on mmap timer timout, this function checks whether the
+ * IRQ is still active and if not, restores the fast path.
+ * by construction a single eventfd is handled at a time.
+ * if the IRQ is still active, the timer is re-programmed.
+ */
+static void vfio_intp_mmap_enable(void *opaque)
+{
+VFIOINTp *tmp;
+VFIOPlatformDevice *vdev = (VFIOPlatformDevice *)opaque;
+
+qemu_mutex_lock(vdev-intp_mutex);
+

[PATCH v13 06/12] kvm: rename kvm_irqchip_[add, remove]_irqfd_notifier with gsi suffix

2015-04-28 Thread Eric Auger

Anticipating for the introduction of new add/remove functions taking
a qemu_irq parameter, let's rename existing ones with a gsi suffix.

Signed-off-by: Eric Auger eric.au...@linaro.org
---
 hw/s390x/virtio-ccw.c  | 8 
 hw/vfio/pci.c  | 6 +++---
 hw/virtio/virtio-pci.c | 4 ++--
 include/sysemu/kvm.h   | 7 ---
 kvm-all.c  | 7 ---
 kvm-stub.c | 7 ---
 6 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ed75c63..55a74fa 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1238,8 +1238,8 @@ static int virtio_ccw_add_irqfd(VirtioCcwDevice *dev, int 
n)
 VirtQueue *vq = virtio_get_queue(vdev, n);
 EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
 
-return kvm_irqchip_add_irqfd_notifier(kvm_state, notifier, NULL,
-  dev-routes.gsi[n]);
+return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, notifier, NULL,
+  dev-routes.gsi[n]);
 }
 
 static void virtio_ccw_remove_irqfd(VirtioCcwDevice *dev, int n)
@@ -1249,8 +1249,8 @@ static void virtio_ccw_remove_irqfd(VirtioCcwDevice *dev, 
int n)
 EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
 int ret;
 
-ret = kvm_irqchip_remove_irqfd_notifier(kvm_state, notifier,
-dev-routes.gsi[n]);
+ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, notifier,
+dev-routes.gsi[n]);
 assert(ret == 0);
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index cd15b20..938f584 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -596,7 +596,7 @@ static void vfio_add_kvm_msi_virq(VFIOMSIVector *vector, 
MSIMessage *msg,
 return;
 }
 
-if (kvm_irqchip_add_irqfd_notifier(kvm_state, vector-kvm_interrupt,
+if (kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, vector-kvm_interrupt,
NULL, virq)  0) {
 kvm_irqchip_release_virq(kvm_state, virq);
 event_notifier_cleanup(vector-kvm_interrupt);
@@ -608,8 +608,8 @@ static void vfio_add_kvm_msi_virq(VFIOMSIVector *vector, 
MSIMessage *msg,
 
 static void vfio_remove_kvm_msi_virq(VFIOMSIVector *vector)
 {
-kvm_irqchip_remove_irqfd_notifier(kvm_state, vector-kvm_interrupt,
-  vector-virq);
+kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, vector-kvm_interrupt,
+  vector-virq);
 kvm_irqchip_release_virq(kvm_state, vector-virq);
 vector-virq = -1;
 event_notifier_cleanup(vector-kvm_interrupt);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c7c3f72..3be7fad 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -477,7 +477,7 @@ static int kvm_virtio_pci_irqfd_use(VirtIOPCIProxy *proxy,
 VirtQueue *vq = virtio_get_queue(vdev, queue_no);
 EventNotifier *n = virtio_queue_get_guest_notifier(vq);
 int ret;
-ret = kvm_irqchip_add_irqfd_notifier(kvm_state, n, NULL, irqfd-virq);
+ret = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd-virq);
 return ret;
 }
 
@@ -491,7 +491,7 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy 
*proxy,
 VirtIOIRQFD *irqfd = proxy-vector_irqfd[vector];
 int ret;
 
-ret = kvm_irqchip_remove_irqfd_notifier(kvm_state, n, irqfd-virq);
+ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd-virq);
 assert(ret == 0);
 }
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 197e6c0..0f28d6f 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -413,9 +413,10 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_adapter_route(KVMState *s, AdapterInfo *adapter);
 
-int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n,
-   EventNotifier *rn, int virq);
-int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq);
+int kvm_irqchip_add_irqfd_notifier_gsi(KVMState *s, EventNotifier *n,
+   EventNotifier *rn, int virq);
+int kvm_irqchip_remove_irqfd_notifier_gsi(KVMState *s, EventNotifier *n,
+  int virq);
 void kvm_pc_gsi_handler(void *opaque, int n, int level);
 void kvm_pc_setup_irq_routing(bool pci_enabled);
 void kvm_init_irq_routing(KVMState *s);
diff --git a/kvm-all.c b/kvm-all.c
index 4ec153d..42bb923 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1368,14 +1368,15 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, 
MSIMessage msg)
 }
 #endif /* !KVM_CAP_IRQ_ROUTING */
 
-int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n,
-   EventNotifier *rn, int virq)
+int kvm_irqchip_add_irqfd_notifier_gsi(KVMState *s, EventNotifier *n,
+   EventNotifier *rn,

[PATCH v13 02/12] hw/vfio/platform: vfio-platform skeleton

2015-04-28 Thread Eric Auger

Minimal VFIO platform implementation supporting register space
user mapping but not IRQ assignment.

Signed-off-by: Kim Phillips kim.phill...@linaro.org
Signed-off-by: Eric Auger eric.au...@linaro.org

---
v12 - v13:
- check device name does not contain any /
- handle case where readlink fully fills the buffer
- in vfio_map_region declare size as uint64_t

v11 - v12:
- add x-mmap property definition, without which the default value of
  vbasedev.allow_mmap is false, hence preventing the reg space from
  being mapped.

v10 - v11:
x Take into account Alex Bennee's comments:
- use g_malloc0_n instead of g_malloc0
- use block declarations when possible
- rework readlink returned value treatment
- use g_strlcat in place of strncat
x use g_snprintf in place of snprintf
x correct error handling in vfio_populate_device,
  in case of flag not corresponding to platform device
x various cosmetic changes

v9 - v10:
- vfio_populate_device no more called in common vfio_get_device
  but in vfio_base_device_init

v8 - v9:
- irq management is moved into a separate patch to ease the review
- VFIO_DEVICE_FLAGS_PLATFORM is checked in vfio_populate_device
- g_free of regions added in vfio_populate_device error label
- virtualID becomes 32b

v7 - v8:
- change proto of vfio_platform_compute_needs_reset and sets
  vbasedev-needs_reset to false there
- vfio_[un]mask_irqindex renamed into vfio_[un]mask_single_irqindex
- vfio_register_irq_starter renamed into vfio_kick_irqs
  we now use a reset notifier instead of a machine init done notifier.
  Enables to get rid of the VfioIrqStarterNotifierParams dangling
  pointer. Previously we use pbus first_irq. This is no more possible
  since the reset notifier takes a void * and first_irq is a field of
  a const struct. So now we pass the DeviceState handle of the
  interrupt controller. I tried to keep the code generic, reason why
  I did not rely on an architecture specific accessor to retrieve
  the gsi number (gic accessor as proposed by Alex). I would like to
  avoid creating an ARM VFIO device model. I hope this model
  model can work on other archs than arm (no multiple intc?);
  wouldn't it be simpler to keep the previous first_irq parameter and
  relax the const constraint.

v6 - v7:
- compat is not exposed anymore as a user option. Rationale is
  the vfio device became abstract and a specialization is needed
  anyway. The derived device must set the compat string.
- in v6 vfio_start_irq_injection was exposed in vfio-platform.h.
  A new function dubbed vfio_register_irq_starter replaces it. It
  registers a machine init done notifier that programs  starts
  all dynamic VFIO device IRQs. This function is supposed to be
  called by the machine file. A set of static helper routines are
  added too. It must be called before the creation of the platform
  bus device.

v5 - v6:
- vfio_device property renamed into host property
- correct error handling of VFIO_DEVICE_GET_IRQ_INFO ioctl
  and remove PCI related comment
- remove declaration of vfio_setup_irqfd and irqfd_allowed
  property.Both belong to next patch (irqfd)
- remove declaration of vfio_intp_interrupt in vfio-platform.h
- functions that can be static get this characteristic
- remove declarations of vfio_region_ops, vfio_memory_listener,
  group_list, vfio_address_spaces. All are moved to vfio-common.h
- remove vfio_put_device declaration and definition
- print_regions removed. code moved into vfio_populate_regions
- replace DPRINTF by trace events
- new helper routine to set the trigger eventfd
- dissociate intp init from the injection enablement:
  vfio_enable_intp renamed into vfio_init_intp and new function
  named vfio_start_eventfd_injection
- injection start moved to vfio_start_irq_injection (not anymore
  in vfio_populate_interrupt)
- new start_irq_fn field in VFIOPlatformDevice corresponding to
  the function that will be used for starting injection
- user handled eventfd:
  x add mutex to protect IRQ state  list manipulation,
  x correct misleading comment in vfio_intp_interrupt.
  x Fix bugs thanks to fake interrupt modality
- VFIOPlatformDeviceClass becomes abstract
- add error_setg in vfio_platform_realize

v4 - v5:
- vfio-plaform.h included first
- cleanup error handling in *populate*, vfio_get_device,
  vfio_enable_intp
- vfio_put_device not called anymore
- add some includes to follow vfio policy

v3 - v4:
[Eric Auger]
- merge of vfio: Add initial IRQ support in platform device
  to get a full functional patch although perfs are limited.
- removal of unrealize function since I currently understand
  it is only used with device hot-plug feature.

v2 - v3:
[Eric Auger]
- further factorization between PCI and platform (VFIORegion,
  VFIODevice). same level of functionality.

= v2:
[Kim Philipps]
- Initial Creation of the device supporting register space mapping
---
 hw/vfio/Makefile.objs   |   1 +
 hw/vfio/platform.c  | 288

[PATCH v13 08/12] intc: arm_gic_kvm: set the qemu_irq/gsi mapping

2015-04-28 Thread Eric Auger

The arm_gic_kvm now calls kvm_irqchip_set_qemuirq_gsi to build
the hash table storing qemu_irq/gsi mappings. From that point on
irqfd can be setup directly from the qemu_irq using
kvm_irqchip_add_irqfd_notifier.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v2 - v3:
- kvm_irqchip_add_qemuirq_irqfd_notifier renamed into
  kvm_irqchip_add_irqfd_notifier
---
 hw/intc/arm_gic_kvm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index e1952ad..28506e3 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -554,6 +554,11 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error 
**errp)
  */
 i += (GIC_INTERNAL * s-num_cpu);
 qdev_init_gpio_in(dev, kvm_arm_gic_set_irq, i);
+
+for (i = 0; i  s-num_irq - GIC_INTERNAL; i++) {
+qemu_irq irq = qdev_get_gpio_in(dev, i);
+kvm_irqchip_set_qemuirq_gsi(kvm_state, irq, i);
+}
 /* We never use our outbound IRQ lines but provide them so that
  * we maintain the same interface as the non-KVM GIC.
  */
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

2015-04-28 Thread Alex Bennée


Peter Maydell peter.mayd...@linaro.org writes:

 On 27 April 2015 at 21:04, Christoffer Dall christoffer.d...@linaro.org 
 wrote:
 On Thu, Apr 23, 2015 at 03:26:53PM +0100, Alex Bennée wrote:

 Christoffer Dall christoffer.d...@linaro.org writes:

  On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote:
  + * just need to report the PC and the HSR values to userspace.
  + * Userspace may decide to re-inject the exception and deliver it to
  + * the guest if it wasn't for the host to deal with.
 
  now I'm confused - does userspace setup the guest to receive an
  exception or does it tell KVM to emulate an exception for the guest or
  do we execute the breakpoint without trapping the debug exception?

 I've made it all go through userspace as we may have to translate the
 hypervisor visible exception code to what the guest was expecting to see.


 ok, so I think you should re-phrase something like:

 Userspace may decide that this exception is caused by the guest using
 debugging itself, and may in that case emulate the guest debug exception
 in userspace before resuming KVM.

 But does that really work?  Given that we don't support KVM-TCG
 migration, this sounds a little strange.  Did we test it?

 The QEMU patches have a TODO note at the point where you'd want
 to do this... Design-wise you can do the reinjection in the
 kernel or in userspace (ppc QEMU does this in userspace, for
 instance) because it's pretty much just setting registers to fake
 up the exception-entry into EL1. Code-wise QEMU's ARM code isn't
 set up to do it right now, but it shouldn't be too difficult to
 persuade the TCG exception-entry code to work for this case too.

 Does the kernel already have a conveniently implemented inject
 exception into guest lump of code? If so it might be less effort
 to do it that way round, maybe.

So you pointed out we can't just re-inject the exceptions we get as we
need to map from things like ESR_ELx_EC_WATCHPT_LOW to
ESR_ELx_EC_WATCHPT_CUR before re-injection.

Of course if it is as simple as modifying the ESR_EL1 register and
returning +ve in the handle_exit path then I can do that but I assumed
if any other wrangling needs doing it should be done in userspace.


 -- PMM

-- 
Alex Bennée
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

2015-04-28 Thread Eric Auger

On 04/28/2015 08:57 AM, Peter Crosthwaite wrote:
 On Mon, Apr 27, 2015 at 11:46 PM, Eric Auger eric.au...@linaro.org wrote:
 Hi Peter,

 On 04/27/2015 07:43 PM, Peter Crosthwaite wrote:
 On Mon, Apr 27, 2015 at 8:01 AM, Paolo Bonzini pbonz...@redhat.com wrote:


 On 27/04/2015 16:56, Eric Auger wrote:
 Peter, Paolo,

 After your feedbacks, I feel I need to spend some more time on the
 original check() track. I would prefer not to introduce any patch that
 will make issue in the future.

 Peter, see the other threads between me and Eric.  See in particular
 http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02749.html
 starting at The notifier actually is not even necessary and the
 replies from there.


 Thanks,

 I see the problem with check. In this reply

 http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02956.html

 Eric says that the problem with the check hook is it happens before
 the setting. I think this can be solved with a RYO link setter for
 GPIOs. We almost have an in-tree precedent with MemoryRegion and the
 container property (memory.c):

  960 op = object_property_add(OBJECT(mr), container,
  961  link TYPE_MEMORY_REGION ,
  962  memory_region_get_container,
  963  NULL, /* memory_region_set_container */
  964  NULL, NULL, error_abort);

 Now in reality we could have done this link normal style as it is only
 a trivial getter, but the reason the link was done this way in the
 first place, is because I have a follow up patch to memory.c that adds
 a customer Link setter:

 +static void memory_region_set_container(Object *obj, Visitor *v, void 
 *opaque,
 +const char *name, Error **errp)
 +{
 +MemoryRegion *mr = MEMORY_REGION(obj);
 +Error *local_err = NULL;
 +MemoryRegion *old_container = mr-container;
 +MemoryRegion *new_container = NULL;
 +char *path = NULL;
 +
 +visit_type_str(v, path, name, local_err);
 +
 +if (!local_err  strcmp(path, ) != 0) {
 +new_container = MEMORY_REGION(object_resolve_link(obj, name, path,
 +  local_err));
 +while (new_container-alias) {
 +new_container = new_container-alias;
 +}
 +}
 +
 +if (local_err) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +object_ref(OBJECT(new_container));
 +
 +memory_region_transaction_begin();
 +memory_region_ref(mr);
 +if (old_container) {
 +memory_region_del_subregion(old_container, mr);
 +}
 +mr-container = new_container;
 +if (new_container) {
 +memory_region_update_container_subregions(mr);
 +}
 +memory_region_unref(mr);
 +memory_region_transaction_commit();
 +
 +object_unref(OBJECT(old_container));
 +}
 +

  op = object_property_add(OBJECT(mr), container,
   link TYPE_MEMORY_REGION ,
   memory_region_get_container,
 - NULL, /* memory_region_set_container */
 + memory_region_set_container,
   NULL, NULL, error_abort);


 The function does the normal link setting - similar to
 object_set_link_property (not to be confused with
 object_property_set_link!) but is surrounded by class specific side
 effects. Specifically in this case, it does
 memory_region_transaction_begin/ref/unref/commit etc for the MR.

 For this GPIO case, you could create a custom setter that does the
 normal set, then calls the DeviceClass installed hook (or you can
 install the hook to the object and init it at qdev_init_gpio_out_named
 time as suggested in the eariler thread). The callback will happen
 after the link is populated.

 To reduce verbosity, I suggest making object_set_link_property() a
 visible API, then RYO link setters can call it surrounded by custom
 behavior e.g:

 foo_object_set_bar_property(...)
 {
 pre_set_link_side_effects();
 object_set_link_property();
 post_set_link_side_effects();
 }

 object_set_link_property() would need to be coreified and wrapped to
 remove it's awareness of LinkProperty type (as that doesn't exist in
 RYO properties) in this case.

 Thank you Peter for detailing this.

 Yesterday I re-worked on the solution based on the check() method where
 - check would take a Object **child as a 3d parameter
 - we would assign *child before the call and in case the check fails set
 the *child back to NULL in object_set_link_property.

 
 I think this is starting to reach outside the design intent of the
 check, letting it be an API that takes over the responsibility of
 -set. Ideally check should be boolean and side-effectless.
I acknowledge ;-)
 
 I need to do some more testing.

 I don't know if this solution would be acceptable too. If not I will
 implement according to your guidelines.

 
 Lets see what

Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

2015-04-28 Thread Eric Auger

Hi Peter,

On 04/27/2015 07:43 PM, Peter Crosthwaite wrote:
 On Mon, Apr 27, 2015 at 8:01 AM, Paolo Bonzini pbonz...@redhat.com wrote:


 On 27/04/2015 16:56, Eric Auger wrote:
 Peter, Paolo,

 After your feedbacks, I feel I need to spend some more time on the
 original check() track. I would prefer not to introduce any patch that
 will make issue in the future.

 Peter, see the other threads between me and Eric.  See in particular
 http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02749.html
 starting at The notifier actually is not even necessary and the
 replies from there.

 
 Thanks,
 
 I see the problem with check. In this reply
 
 http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02956.html
 
 Eric says that the problem with the check hook is it happens before
 the setting. I think this can be solved with a RYO link setter for
 GPIOs. We almost have an in-tree precedent with MemoryRegion and the
 container property (memory.c):
 
  960 op = object_property_add(OBJECT(mr), container,
  961  link TYPE_MEMORY_REGION ,
  962  memory_region_get_container,
  963  NULL, /* memory_region_set_container */
  964  NULL, NULL, error_abort);
 
 Now in reality we could have done this link normal style as it is only
 a trivial getter, but the reason the link was done this way in the
 first place, is because I have a follow up patch to memory.c that adds
 a customer Link setter:
 
 +static void memory_region_set_container(Object *obj, Visitor *v, void 
 *opaque,
 +const char *name, Error **errp)
 +{
 +MemoryRegion *mr = MEMORY_REGION(obj);
 +Error *local_err = NULL;
 +MemoryRegion *old_container = mr-container;
 +MemoryRegion *new_container = NULL;
 +char *path = NULL;
 +
 +visit_type_str(v, path, name, local_err);
 +
 +if (!local_err  strcmp(path, ) != 0) {
 +new_container = MEMORY_REGION(object_resolve_link(obj, name, path,
 +  local_err));
 +while (new_container-alias) {
 +new_container = new_container-alias;
 +}
 +}
 +
 +if (local_err) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +object_ref(OBJECT(new_container));
 +
 +memory_region_transaction_begin();
 +memory_region_ref(mr);
 +if (old_container) {
 +memory_region_del_subregion(old_container, mr);
 +}
 +mr-container = new_container;
 +if (new_container) {
 +memory_region_update_container_subregions(mr);
 +}
 +memory_region_unref(mr);
 +memory_region_transaction_commit();
 +
 +object_unref(OBJECT(old_container));
 +}
 +
 
  op = object_property_add(OBJECT(mr), container,
   link TYPE_MEMORY_REGION ,
   memory_region_get_container,
 - NULL, /* memory_region_set_container */
 + memory_region_set_container,
   NULL, NULL, error_abort);
 
 
 The function does the normal link setting - similar to
 object_set_link_property (not to be confused with
 object_property_set_link!) but is surrounded by class specific side
 effects. Specifically in this case, it does
 memory_region_transaction_begin/ref/unref/commit etc for the MR.
 
 For this GPIO case, you could create a custom setter that does the
 normal set, then calls the DeviceClass installed hook (or you can
 install the hook to the object and init it at qdev_init_gpio_out_named
 time as suggested in the eariler thread). The callback will happen
 after the link is populated.
 
 To reduce verbosity, I suggest making object_set_link_property() a
 visible API, then RYO link setters can call it surrounded by custom
 behavior e.g:
 
 foo_object_set_bar_property(...)
 {
 pre_set_link_side_effects();
 object_set_link_property();
 post_set_link_side_effects();
 }
 
 object_set_link_property() would need to be coreified and wrapped to
 remove it's awareness of LinkProperty type (as that doesn't exist in
 RYO properties) in this case.

Thank you Peter for detailing this.

Yesterday I re-worked on the solution based on the check() method where
- check would take a Object **child as a 3d parameter
- we would assign *child before the call and in case the check fails set
the *child back to NULL in object_set_link_property.

I need to do some more testing.

I don't know if this solution would be acceptable too. If not I will
implement according to your guidelines.

Best Regards

Eric
 
 Regards,
 Peter
 
 If you have any idea, please help.

 Paolo


___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

2015-04-28 Thread Christian Borntraeger

Some architectures already have irq disabled when calling
kvm_guest_exit. Push down the disabling into the architectures
to avoid double disabling. This also allows to replace
irq_save with irq_disable which might be cheaper.
arm and mips already have interrupts disabled. s390/power/x86
need adoptions.

Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
 arch/powerpc/kvm/book3s_hv.c | 2 ++
 arch/powerpc/kvm/book3s_pr.c | 2 ++
 arch/powerpc/kvm/booke.c | 4 ++--
 arch/s390/kvm/kvm-s390.c | 2 ++
 arch/x86/kvm/x86.c   | 2 ++
 include/linux/kvm_host.h | 4 
 6 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a5f392d..63b35b1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1811,7 +1811,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
 
/* make sure updates to secondary vcpu structs are visible now */
smp_mb();
+   local_irq_disable();
kvm_guest_exit();
+   local_irq_enable();
 
preempt_enable();
cond_resched();
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index f573839..eafcb8c 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
 
/* We get here with MSR.EE=1 */
 
+   local_irq_disable();
trace_kvm_exit(exit_nr, vcpu);
+   local_irq_enable();
kvm_guest_exit();
 
switch (exit_nr) {
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 6c1316a..f1d6af3 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1004,11 +1004,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
break;
}
 
-   local_irq_enable();
-
trace_kvm_exit(exit_nr, vcpu);
kvm_guest_exit();
 
+   local_irq_enable();
+
run-exit_reason = KVM_EXIT_UNKNOWN;
run-ready_for_interrupt_injection = 1;
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 9f4c954..0aa2534 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2015,7 +2015,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
local_irq_enable();
exit_reason = sie64a(vcpu-arch.sie_block,
 vcpu-run-s.regs.gprs);
+   local_irq_disable();
kvm_guest_exit();
+   local_irq_enable();
vcpu-srcu_idx = srcu_read_lock(vcpu-kvm-srcu);
 
rc = vcpu_post_run(vcpu, exit_reason);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32bf19e..a5fbd45 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6351,7 +6351,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 */
barrier();
 
+   local_irq_disable();
kvm_guest_exit();
+   local_irq_enable();
 
preempt_enable();
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a34bf6ed..fe699fb 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void)
 
 static inline void kvm_guest_exit(void)
 {
-   unsigned long flags;
-
-   local_irq_save(flags);
guest_exit();
-   local_irq_restore(flags);
 }
 
 /*
-- 
2.3.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling

2015-04-28 Thread Christian Borntraeger

I was able to get rid of some nanoseconds for a guest exit loop
on s390. I did my best to not break other architectures but
review and comments on the general approach is welcome.
Downside is that the existing irq_save things will just work
no matter what the callers have done, the new code must do
the right thing in the callers.

Is that approach acceptible? Does anybody else see some measurable
difference for guest exits?


Christian Borntraeger (2):
  KVM: Push down irq_save to architectures before kvm_guest_enter
  KVM: push down irq_save from kvm_guest_exit

 arch/powerpc/kvm/book3s_hv.c |  4 
 arch/powerpc/kvm/book3s_pr.c |  2 ++
 arch/powerpc/kvm/booke.c |  4 ++--
 arch/s390/kvm/kvm-s390.c |  6 --
 arch/x86/kvm/x86.c   |  2 ++
 include/linux/kvm_host.h | 18 --
 6 files changed, 22 insertions(+), 14 deletions(-)

-- 
2.3.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter

2015-04-28 Thread Christian Borntraeger

local_irq_disable can be cheaper than local_irq_save, especially
when done only once instead of twice. We can push down the
local_irq_save (and replace it with local_irq_disable) to
save some cycles.
x86, mips and arm already disable the interrupts before calling
kvm_guest_enter. Here we save one local_irq_save/restore pair.
power and s390 are reworked to disable the interrupts before calling
kvm_guest_enter. s390 saves a preempt_disable/enable pair but also
saves some cycles as local_irq_disable/enable can be cheaper than
local_irq_save/restore on some machines.

power should be almost a no-op change (interrupts are disabled
slighty longer).

Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
 arch/powerpc/kvm/book3s_hv.c |  2 ++
 arch/s390/kvm/kvm-s390.c |  4 ++--
 include/linux/kvm_host.h | 14 --
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index de74756..a5f392d 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1779,7 +1779,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
 
spin_unlock(vc-lock);
 
+   local_irq_disable();
kvm_guest_enter();
+   local_irq_enable();
 
srcu_idx = srcu_read_lock(vc-kvm-srcu);
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 46f37df..9f4c954 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2010,9 +2010,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
 * As PF_VCPU will be used in fault handler, between
 * guest_enter and guest_exit should be no uaccess.
 */
-   preempt_disable();
+   local_irq_disable();
kvm_guest_enter();
-   preempt_enable();
+   local_irq_enable();
exit_reason = sie64a(vcpu-arch.sie_block,
 vcpu-run-s.regs.gprs);
kvm_guest_exit();
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d12b210..a34bf6ed 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -751,13 +751,15 @@ static inline void kvm_iommu_unmap_pages(struct kvm *kvm,
 
 static inline void kvm_guest_enter(void)
 {
-   unsigned long flags;
-
-   BUG_ON(preemptible());
-
-   local_irq_save(flags);
+   /*
+* guest_enter needs disabled irqs and rcu_virt_note_context_switch
+* wants disabled preemption. Ensure that the caller has disabled
+* irqs for kvm_guest_enter. Please note: Some architectures (e.g.
+* s390) will reenable irqs before entering the guest, but this is
+* ok. We just need a stable CPU for the accounting itself.
+*/
+   WARN_ON(!irqs_disabled());
guest_enter();
-   local_irq_restore(flags);
 
/* KVM does not hold any references to rcu protected data when it
 * switches CPU into a guest mode. In fact switching to a guest mode
-- 
2.3.0

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/3] qdev: check callback takes Object **target as third argument

2015-04-28 Thread Eric Auger

Check callback now takes as third argument an Object **. In
object_set_link_property, we pass the property child as argument.
We also assign the *child before the check call so that enhanced
check can be performed in the callback. In case the check fails,
the old value is restored and ref count is left unchanged.

This typically makes possible to do checks both on the *child
content (for instance a qemu_irq) and also perform some actions/
checks on its container, which was not possible before.

This is typically useful for starting irqfd setup in vfio platform
use case.

Signed-off-by: Eric Auger eric.au...@linaro.org
---
 hw/core/qdev-properties.c|  2 +-
 include/hw/qdev-properties.h |  2 +-
 include/qom/object.h |  7 +++
 qom/object.c | 15 +--
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 570d5f0..a42f9d4 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -22,7 +22,7 @@ void qdev_prop_set_after_realize(DeviceState *dev, const char 
*name,
 }
 
 void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
- Object *val, Error **errp)
+ Object **target, Error **errp)
 {
 DeviceState *dev = DEVICE(obj);
 
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index d67dad5..b2868be 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -213,6 +213,6 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
char *name,
  * object_property_add_link().
  */
 void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
- Object *val, Error **errp);
+ Object **target, Error **errp);
 
 #endif
diff --git a/include/qom/object.h b/include/qom/object.h
index 4687fa1..0a7daff 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -34,7 +34,7 @@ typedef struct InterfaceClass InterfaceClass;
 typedef struct InterfaceInfo InterfaceInfo;
 
 typedef void (*object_property_set_link_t)(Object *, const char *,
-   Object *, Error **);
+   Object **, Error **);
 
 #define TYPE_OBJECT object
 
@@ -1136,7 +1136,7 @@ typedef enum {
  * an error.
  */
 void object_property_allow_set_link(Object *, const char *,
-Object *, Error **);
+Object **, Error **);
 
 /**
  * object_property_add_link:
@@ -1168,8 +1168,7 @@ void object_property_allow_set_link(Object *, const char 
*,
  */
 void object_property_add_link(Object *obj, const char *name,
   const char *type, Object **child,
-  void (*check)(Object *obj, const char *name,
-Object *val, Error **errp),
+  object_property_set_link_t check,
   ObjectPropertyLinkFlags flags,
   Error **errp);
 
diff --git a/qom/object.c b/qom/object.c
index b8dff43..cc9ed87 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1112,14 +1112,14 @@ out:
 }
 
 void object_property_allow_set_link(Object *obj, const char *name,
-Object *val, Error **errp)
+Object **target, Error **errp)
 {
 /* Allow the link to be set, always */
 }
 
 typedef struct {
 Object **child;
-void (*check)(Object *, const char *, Object *, Error **);
+void (*check)(Object *, const char *, Object **, Error **);
 ObjectPropertyLinkFlags flags;
 } LinkProperty;
 
@@ -1201,14 +1201,17 @@ static void object_set_link_property(Object *obj, 
Visitor *v, void *opaque,
 return;
 }
 
-prop-check(obj, name, new_target, local_err);
+object_ref(new_target);
+*child = new_target;
+
+prop-check(obj, name, child, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
+*child = old_target;
+object_ref(new_target);
 return;
 }
 
-object_ref(new_target);
-*child = new_target;
 object_unref(old_target);
 }
 
@@ -1233,7 +1236,7 @@ static void object_release_link_property(Object *obj, 
const char *name,
 void object_property_add_link(Object *obj, const char *name,
   const char *type, Object **child,
   void (*check)(Object *, const char *,
-Object *, Error **),
+Object **, Error **),
   ObjectPropertyLinkFlags flags,
   Error **errp)
 {
-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu

[PATCH 0/3] Extend check callback usability for irq connect notifier

2015-04-28 Thread Eric Auger

The VFIO platform device gets connected to the platform bus
on a machine init done notifier. Only at that point irqfd can be
setup. An irq connect notifier would be helpful to do that job.
Instead of adding a new callback at sysbus or qdev level, this
series proposes to use the property check() callback (credit to
Paolo).

- the callback is passed to qdev_init_gpio_out_named
- sysbus class now holds a irq_set_hook method usable as check
  callback
- sysbus_init_irq initializes the callback to the class method
- object_set_link_property is modified so that
  * the target object is populated before the check
  * check takes an Object ** enabling to access the object
container.

Then The VFIO platform device would override the irq_set_hook
method in a separate patch.

Please let me know if those modifications are acceptable. Else
I will follow Peter's proposal. 

Best Regards

Eric

Eric Auger (3):
  qdev: pass the check callback to qdev_init_gpio_out_named
  qdev: check callback takes Object **target as third argument
  sysbus: add irq_set_hook

 hw/core/qdev-properties.c|  2 +-
 hw/core/qdev.c   |  8 +---
 hw/core/sysbus.c |  8 +++-
 include/hw/qdev-core.h   |  3 ++-
 include/hw/qdev-properties.h |  2 +-
 include/hw/sysbus.h  |  1 +
 include/qom/object.h |  8 +---
 qom/object.c | 15 +--
 8 files changed, 31 insertions(+), 16 deletions(-)

-- 
1.8.3.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

Re: [PATCH 2/3] qdev: check callback takes Object **target as third argument

[PATCH v13 01/12] linux-headers: update headers according to 4.1-rc0

[PATCH v13 10/12] qdev: check callback takes the property child as third argument

[PATCH v13 11/12] sysbus: add irq_set_hook

[PATCH v13 04/12] hw/vfio/platform: calxeda xgmac device

[PATCH v13 05/12] hw/arm/sysbus-fdt: enable vfio-calxeda-xgmac dynamic instantiation

[PATCH v13 07/12] kvm-all.c: add qemu_irq/gsi hash table and utility routines

[PATCH v13 09/12] qdev: pass the check callback to qdev_init_gpio_out_named

[PATCH v13 03/12] hw/vfio/platform: add irq assignment

[PATCH v13 06/12] kvm: rename kvm_irqchip_[add, remove]_irqfd_notifier with gsi suffix

[PATCH v13 02/12] hw/vfio/platform: vfio-platform skeleton

[PATCH v13 08/12] intc: arm_gic_kvm: set the qemu_irq/gsi mapping

Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support

Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

[PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

[PATCH/RFC 0/2] KVM: micro-optimization and interrupt disabling

[PATCH/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter

[PATCH 2/3] qdev: check callback takes Object **target as third argument

[PATCH 0/3] Extend check callback usability for irq connect notifier

24 matches

Site Navigation

Mail list logo

Footer information