Re: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

2015-01-26 Thread Andre Przywara 
Hi Will,

On 26/01/15 11:30, Will Deacon wrote:
 On Fri, Jan 23, 2015 at 04:35:10PM +, Andre Przywara wrote:
 Add the command line parameter --gicv3 to request GICv3 emulation
 in the kernel. Connect that to the already existing GICv3 code.

 Signed-off-by: Andre Przywara andre.przyw...@arm.com
 ---
  tools/kvm/arm/aarch64/arm-cpu.c|5 -
  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |4 +++-
  tools/kvm/arm/gic.c|   14 ++
  tools/kvm/arm/include/arm-common/kvm-config-arch.h |1 +
  tools/kvm/arm/kvm-cpu.c|2 +-
  tools/kvm/arm/kvm.c|3 ++-
  6 files changed, 25 insertions(+), 4 deletions(-)

 diff --git a/tools/kvm/arm/aarch64/arm-cpu.c 
 b/tools/kvm/arm/aarch64/arm-cpu.c
 index a70d6bb..46d6d6a 100644
 --- a/tools/kvm/arm/aarch64/arm-cpu.c
 +++ b/tools/kvm/arm/aarch64/arm-cpu.c
 @@ -12,7 +12,10 @@
  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
  {
  int timer_interrupts[4] = {13, 14, 11, 10};
 -gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
 +gic__generate_fdt_nodes(fdt, gic_phandle,
 +kvm-cfg.arch.gicv3 ?
 +KVM_DEV_TYPE_ARM_VGIC_V3 :
 +KVM_DEV_TYPE_ARM_VGIC_V2);
  timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
  }
  
 diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h 
 b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 index 89860ae..106e52f 100644
 --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 @@ -3,7 +3,9 @@
  
  #define ARM_OPT_ARCH_RUN(cfg)   
 \
  OPT_BOOLEAN('\0', aarch32, (cfg)-aarch32_guest, \
 -Run AArch32 guest),
 +Run AArch32 guest),   \
 +OPT_BOOLEAN('\0', gicv3, (cfg)-gicv3,   \
 +Use a GICv3 interrupt controller in the guest),
 
 On a GICv3-capable system, why would I *not* want to enable this option?
 In other words, could we make this the default behaviour on systems that
 support it, and if you need an override then it should be something like
 --force-gicv2.

Well, you could have a guest kernel  3.17, which does not have GICv3
support. In general I consider GICv2 better tested, so I reckon that
people will only want to use GICv3 emulation if there is a need for it
(non-compat GICv3 host or more than 8 VCPUs in the guest). That probably
changes over time, but for the time being I'd better keep the default at
GICv2 emulation.

Having said that, there could be a fallback in case GICv2 emulation is
not available. Let me take a look at that.
Also thinking about the future (ITS emulation) I found that I'd like to
replace this option with something more generic like --irqchip=.

Cheers,
Andre.
--
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: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

2015-01-26 Thread Will Deacon 
On Fri, Jan 23, 2015 at 04:35:10PM +, Andre Przywara wrote:
 Add the command line parameter --gicv3 to request GICv3 emulation
 in the kernel. Connect that to the already existing GICv3 code.
 
 Signed-off-by: Andre Przywara andre.przyw...@arm.com
 ---
  tools/kvm/arm/aarch64/arm-cpu.c|5 -
  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |4 +++-
  tools/kvm/arm/gic.c|   14 ++
  tools/kvm/arm/include/arm-common/kvm-config-arch.h |1 +
  tools/kvm/arm/kvm-cpu.c|2 +-
  tools/kvm/arm/kvm.c|3 ++-
  6 files changed, 25 insertions(+), 4 deletions(-)
 
 diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
 index a70d6bb..46d6d6a 100644
 --- a/tools/kvm/arm/aarch64/arm-cpu.c
 +++ b/tools/kvm/arm/aarch64/arm-cpu.c
 @@ -12,7 +12,10 @@
  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
  {
   int timer_interrupts[4] = {13, 14, 11, 10};
 - gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
 + gic__generate_fdt_nodes(fdt, gic_phandle,
 + kvm-cfg.arch.gicv3 ?
 + KVM_DEV_TYPE_ARM_VGIC_V3 :
 + KVM_DEV_TYPE_ARM_VGIC_V2);
   timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
  }
  
 diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h 
 b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 index 89860ae..106e52f 100644
 --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 @@ -3,7 +3,9 @@
  
  #define ARM_OPT_ARCH_RUN(cfg)
 \
   OPT_BOOLEAN('\0', aarch32, (cfg)-aarch32_guest, \
 - Run AArch32 guest),
 + Run AArch32 guest),   \
 + OPT_BOOLEAN('\0', gicv3, (cfg)-gicv3,   \
 + Use a GICv3 interrupt controller in the guest),

On a GICv3-capable system, why would I *not* want to enable this option?
In other words, could we make this the default behaviour on systems that
support it, and if you need an override then it should be something like
--force-gicv2.

Or am I missing a key piece of the puzzle?

Will
--
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: [PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

2015-01-26 Thread Marc Zyngier 
On 26/01/15 11:43, Andre Przywara wrote:
 Hi Will,
 
 On 26/01/15 11:30, Will Deacon wrote:
 On Fri, Jan 23, 2015 at 04:35:10PM +, Andre Przywara wrote:
 Add the command line parameter --gicv3 to request GICv3 emulation
 in the kernel. Connect that to the already existing GICv3 code.

 Signed-off-by: Andre Przywara andre.przyw...@arm.com
 ---
  tools/kvm/arm/aarch64/arm-cpu.c|5 -
  .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |4 +++-
  tools/kvm/arm/gic.c|   14 ++
  tools/kvm/arm/include/arm-common/kvm-config-arch.h |1 +
  tools/kvm/arm/kvm-cpu.c|2 +-
  tools/kvm/arm/kvm.c|3 ++-
  6 files changed, 25 insertions(+), 4 deletions(-)

 diff --git a/tools/kvm/arm/aarch64/arm-cpu.c 
 b/tools/kvm/arm/aarch64/arm-cpu.c
 index a70d6bb..46d6d6a 100644
 --- a/tools/kvm/arm/aarch64/arm-cpu.c
 +++ b/tools/kvm/arm/aarch64/arm-cpu.c
 @@ -12,7 +12,10 @@
  static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
  {
 int timer_interrupts[4] = {13, 14, 11, 10};
 -   gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
 +   gic__generate_fdt_nodes(fdt, gic_phandle,
 +   kvm-cfg.arch.gicv3 ?
 +   KVM_DEV_TYPE_ARM_VGIC_V3 :
 +   KVM_DEV_TYPE_ARM_VGIC_V2);
 timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
  }
  
 diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h 
 b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 index 89860ae..106e52f 100644
 --- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 +++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
 @@ -3,7 +3,9 @@
  
  #define ARM_OPT_ARCH_RUN(cfg)  
 \
 OPT_BOOLEAN('\0', aarch32, (cfg)-aarch32_guest, \
 -   Run AArch32 guest),
 +   Run AArch32 guest),   \
 +   OPT_BOOLEAN('\0', gicv3, (cfg)-gicv3,   \
 +   Use a GICv3 interrupt controller in the guest),

 On a GICv3-capable system, why would I *not* want to enable this option?
 In other words, could we make this the default behaviour on systems that
 support it, and if you need an override then it should be something like
 --force-gicv2.
 
 Well, you could have a guest kernel  3.17, which does not have GICv3
 support. In general I consider GICv2 better tested, so I reckon that
 people will only want to use GICv3 emulation if there is a need for it
 (non-compat GICv3 host or more than 8 VCPUs in the guest). That probably
 changes over time, but for the time being I'd better keep the default at
 GICv2 emulation.

I think there is slightly more to it. You want the same command-line
options to give you the same result on different platform (provided that
the HW is available, see below). Changing the default depending on the
platform you're is not very good for reproducibility.

 Having said that, there could be a fallback in case GICv2 emulation is
 not available. Let me take a look at that.

You could try and pick a GICv3 emulation if v2 is not available, and
probably print a warning in that case.

 Also thinking about the future (ITS emulation) I found that I'd like to
 replace this option with something more generic like --irqchip=.

That's an orthogonal issue, but yes, this is probably better.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
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

[PATCH 11/11] kvmtool: add command line parameter to instantiate a vGICv3

2015-01-23 Thread Andre Przywara 
Add the command line parameter --gicv3 to request GICv3 emulation
in the kernel. Connect that to the already existing GICv3 code.

Signed-off-by: Andre Przywara andre.przyw...@arm.com
---
 tools/kvm/arm/aarch64/arm-cpu.c|5 -
 .../kvm/arm/aarch64/include/kvm/kvm-config-arch.h  |4 +++-
 tools/kvm/arm/gic.c|   14 ++
 tools/kvm/arm/include/arm-common/kvm-config-arch.h |1 +
 tools/kvm/arm/kvm-cpu.c|2 +-
 tools/kvm/arm/kvm.c|3 ++-
 6 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c
index a70d6bb..46d6d6a 100644
--- a/tools/kvm/arm/aarch64/arm-cpu.c
+++ b/tools/kvm/arm/aarch64/arm-cpu.c
@@ -12,7 +12,10 @@
 static void generate_fdt_nodes(void *fdt, struct kvm *kvm, u32 gic_phandle)
 {
int timer_interrupts[4] = {13, 14, 11, 10};
-   gic__generate_fdt_nodes(fdt, gic_phandle, KVM_DEV_TYPE_ARM_VGIC_V2);
+   gic__generate_fdt_nodes(fdt, gic_phandle,
+   kvm-cfg.arch.gicv3 ?
+   KVM_DEV_TYPE_ARM_VGIC_V3 :
+   KVM_DEV_TYPE_ARM_VGIC_V2);
timer__generate_fdt_nodes(fdt, kvm, timer_interrupts);
 }
 
diff --git a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h 
b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
index 89860ae..106e52f 100644
--- a/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
+++ b/tools/kvm/arm/aarch64/include/kvm/kvm-config-arch.h
@@ -3,7 +3,9 @@
 
 #define ARM_OPT_ARCH_RUN(cfg)  \
OPT_BOOLEAN('\0', aarch32, (cfg)-aarch32_guest, \
-   Run AArch32 guest),
+   Run AArch32 guest),   \
+   OPT_BOOLEAN('\0', gicv3, (cfg)-gicv3,   \
+   Use a GICv3 interrupt controller in the guest),
 
 #include arm-common/kvm-config-arch.h
 
diff --git a/tools/kvm/arm/gic.c b/tools/kvm/arm/gic.c
index 9435715..3825fab 100644
--- a/tools/kvm/arm/gic.c
+++ b/tools/kvm/arm/gic.c
@@ -95,6 +95,17 @@ static int gic__create_irqchip(struct kvm *kvm)
return err;
 }
 
+static int gicv3__create_irqchip(struct kvm *kvm)
+{
+   if (kvm-nrcpus  255) {
+   pr_warning(%d CPUS greater than maximum of %d -- truncating\n,
+   kvm-nrcpus, 255);
+   kvm-nrcpus = 255;
+   }
+
+   return gic__create_device(kvm, KVM_DEV_TYPE_ARM_VGIC_V3);
+}
+
 static int gicv2__create_irqchip(struct kvm *kvm)
 {
int err;
@@ -118,6 +129,9 @@ int gic__create(struct kvm *kvm, u32 type)
switch (type) {
case KVM_DEV_TYPE_ARM_VGIC_V2:
return gicv2__create_irqchip(kvm);
+   case KVM_DEV_TYPE_ARM_VGIC_V3:
+   nr_redists = kvm-cfg.nrcpus;
+   return gicv3__create_irqchip(kvm);
}
return -ENODEV;
 }
diff --git a/tools/kvm/arm/include/arm-common/kvm-config-arch.h 
b/tools/kvm/arm/include/arm-common/kvm-config-arch.h
index a8ebd94..0c27f88 100644
--- a/tools/kvm/arm/include/arm-common/kvm-config-arch.h
+++ b/tools/kvm/arm/include/arm-common/kvm-config-arch.h
@@ -8,6 +8,7 @@ struct kvm_config_arch {
unsigned intforce_cntfrq;
boolvirtio_trans_pci;
boolaarch32_guest;
+   boolgicv3;
 };
 
 #define OPT_ARCH_RUN(pfx, cfg) 
\
diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index a3344fa..f94fd6e 100644
--- a/tools/kvm/arm/kvm-cpu.c
+++ b/tools/kvm/arm/kvm-cpu.c
@@ -142,7 +142,7 @@ bool kvm_cpu__handle_exit(struct kvm_cpu *vcpu)
 bool kvm_cpu__emulate_mmio(struct kvm_cpu *vcpu, u64 phys_addr, u8 *data,
   u32 len, u8 is_write)
 {
-   int nr_redists = 0;
+   int nr_redists = vcpu-kvm-cfg.arch.gicv3 ? vcpu-kvm-nrcpus : 0;
 
if (arm_addr_in_virtio_mmio_region(nr_redists, phys_addr)) {
return kvm__emulate_mmio(vcpu, phys_addr, data, len, is_write);
diff --git a/tools/kvm/arm/kvm.c b/tools/kvm/arm/kvm.c
index 8d31fd7..2bccea5 100644
--- a/tools/kvm/arm/kvm.c
+++ b/tools/kvm/arm/kvm.c
@@ -82,6 +82,7 @@ void kvm__arch_init(struct kvm *kvm, const char 
*hugetlbfs_path, u64 ram_size)
MADV_MERGEABLE | MADV_HUGEPAGE);
 
/* Create the virtual GIC. */
-   if (gic__create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2))
+   if (gic__create(kvm, kvm-cfg.arch.gicv3 ?
+   KVM_DEV_TYPE_ARM_VGIC_V3 : KVM_DEV_TYPE_ARM_VGIC_V2))
die(Failed to create virtual GIC);
 }
-- 
1.7.9.5

--
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