| Message ID | 1422030910-17881-12-git-send-email-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 23, 2015 at 04:35:10PM +0000, 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.przywara@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Will, On 26/01/15 11:30, Will Deacon wrote: > On Fri, Jan 23, 2015 at 04:35:10PM +0000, 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.przywara@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 +0000, 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.przywara@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.
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 int force_cntfrq; bool virtio_trans_pci; bool aarch32_guest; + bool gicv3; }; #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"); }
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.przywara@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(-)