Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
On Sat, 29 Aug 2020 at 23:22, Auger Eric wrote:
>
> Hi Haibo,
>
> On 8/7/20 10:10 AM, Haibo Xu wrote:
> > Add a virtual SPE device for virt machine while using PPI
> > 5 for SPE overflow interrupt number.
> >
> > Signed-off-by: Haibo Xu
> > ---
> > hw/arm/virt-acpi-build.c| 3 +++
> > hw/arm/virt.c | 42 +
> > include/hw/acpi/acpi-defs.h | 1 +
> > include/hw/arm/virt.h | 1 +
> > target/arm/cpu.c| 2 ++
> > target/arm/cpu.h| 2 ++
> > target/arm/kvm.c| 6 ++
> > 7 files changed, 57 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 91f0df7b13..5073ba22a5 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms)
> > if (arm_feature(>env, ARM_FEATURE_PMU)) {
> > gicc->performance_interrupt =
> > cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> > }
> > +if (arm_feature(>env, ARM_FEATURE_SPE)) {
> > +gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> > +}
> > if (vms->virt) {
> > gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
> > }
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ecfee362a1..c40819705d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState
> > *vms)
> > }
> > }
> >
> > +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> > +{
> > +CPUState *cpu;
> > +ARMCPU *armcpu;
> > +uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> > +
> > +CPU_FOREACH(cpu) {
> > +armcpu = ARM_CPU(cpu);
> > +if (!arm_feature(>env, ARM_FEATURE_SPE)) {
> > +return;
> > +}
> > +if (kvm_enabled()) {
> > +if (kvm_irqchip_in_kernel()) {
> > +kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> > +}
> > +kvm_arm_spe_init(cpu);
> > +}
> > +}
> > +
> > +if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > +irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> > + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> > + (1 << vms->smp_cpus) - 1);
> > +}
> > +
> > +armcpu = ARM_CPU(qemu_get_cpu(0));
> > +qemu_fdt_add_subnode(vms->fdt, "/spe");
> > +if (arm_feature(>env, ARM_FEATURE_V8)) {
> > +const char compat[] = "arm,statistical-profiling-extension-v1";
> > +qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> > + compat, sizeof(compat));
> > +qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> > + GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ,
> > irqflags);
> > +}
> > +}
> > +
> > static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
> > {
> > DeviceState *dev;
> > @@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
> > qdev_get_gpio_in(vms->gic, ppibase
> > + VIRTUAL_PMU_IRQ));
> >
> > +qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> > +qdev_get_gpio_in(vms->gic, ppibase
> > + + VIRTUAL_SPE_IRQ));
> > +
> > sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> > ARM_CPU_IRQ));
> > sysbus_connect_irq(gicbusdev, i + smp_cpus,
> > qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > @@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
> >
> > fdt_add_pmu_nodes(vms);
> >
> > +fdt_add_spe_nodes(vms);
> > +
> > create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
> >
> > if (vms->secure) {
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 38a42f409a..56a7f38ae4 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
> > uint32_t vgic_interrupt;
> > uint64_t gicr_base_address;
> > uint64_t arm_mpidr;
> > +uint16_t spe_interrupt; /* ACPI 6.3 */
> This does not work for me.
> You miss 2 uint8_t fields inbetween arm_mpdir and spe_interrupt:
> Processor Power Efficiency Class and Reserved.
>
> At the moment arm_spe_acpi_register_device() silently fails on guest
> side since
> gicc->header.length < ACPI_MADT_GICC_SPE
>
> Thanks
>
> Eric
>
Thanks for pointing out this. I will fix it in the next version.
> > } QEMU_PACKED;
> >
> > typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index dff67e1bef..56c83224d2 100644
> > --- a/include/hw/arm/virt.h
> > +++
Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
Hi Haibo,
On 8/7/20 10:10 AM, Haibo Xu wrote:
> Add a virtual SPE device for virt machine while using PPI
> 5 for SPE overflow interrupt number.
>
> Signed-off-by: Haibo Xu
> ---
> hw/arm/virt-acpi-build.c| 3 +++
> hw/arm/virt.c | 42 +
> include/hw/acpi/acpi-defs.h | 1 +
> include/hw/arm/virt.h | 1 +
> target/arm/cpu.c| 2 ++
> target/arm/cpu.h| 2 ++
> target/arm/kvm.c| 6 ++
> 7 files changed, 57 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..5073ba22a5 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> if (arm_feature(>env, ARM_FEATURE_PMU)) {
> gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> }
> +if (arm_feature(>env, ARM_FEATURE_SPE)) {
> +gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> +}
> if (vms->virt) {
> gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
> }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ecfee362a1..c40819705d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState
> *vms)
> }
> }
>
> +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> +{
> +CPUState *cpu;
> +ARMCPU *armcpu;
> +uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> +
> +CPU_FOREACH(cpu) {
> +armcpu = ARM_CPU(cpu);
> +if (!arm_feature(>env, ARM_FEATURE_SPE)) {
> +return;
> +}
> +if (kvm_enabled()) {
> +if (kvm_irqchip_in_kernel()) {
> +kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> +}
> +kvm_arm_spe_init(cpu);
> +}
> +}
> +
> +if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> + (1 << vms->smp_cpus) - 1);
> +}
> +
> +armcpu = ARM_CPU(qemu_get_cpu(0));
> +qemu_fdt_add_subnode(vms->fdt, "/spe");
> +if (arm_feature(>env, ARM_FEATURE_V8)) {
> +const char compat[] = "arm,statistical-profiling-extension-v1";
> +qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> + compat, sizeof(compat));
> +qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> + GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ,
> irqflags);
> +}
> +}
> +
> static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
> {
> DeviceState *dev;
> @@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
> qdev_get_gpio_in(vms->gic, ppibase
> + VIRTUAL_PMU_IRQ));
>
> +qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> +qdev_get_gpio_in(vms->gic, ppibase
> + + VIRTUAL_SPE_IRQ));
> +
> sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> ARM_CPU_IRQ));
> sysbus_connect_irq(gicbusdev, i + smp_cpus,
> qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> @@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
>
> fdt_add_pmu_nodes(vms);
>
> +fdt_add_spe_nodes(vms);
> +
> create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
>
> if (vms->secure) {
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 38a42f409a..56a7f38ae4 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
> uint32_t vgic_interrupt;
> uint64_t gicr_base_address;
> uint64_t arm_mpidr;
> +uint16_t spe_interrupt; /* ACPI 6.3 */
This does not work for me.
You miss 2 uint8_t fields inbetween arm_mpdir and spe_interrupt:
Processor Power Efficiency Class and Reserved.
At the moment arm_spe_acpi_register_device() silently fails on guest
side since
gicc->header.length < ACPI_MADT_GICC_SPE
Thanks
Eric
> } QEMU_PACKED;
>
> typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index dff67e1bef..56c83224d2 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -49,6 +49,7 @@
> #define ARCH_TIMER_NS_EL1_IRQ 14
> #define ARCH_TIMER_NS_EL2_IRQ 10
>
> +#define VIRTUAL_SPE_IRQ 5
> #define VIRTUAL_PMU_IRQ 7
>
> #define PPI(irq) ((irq) + 16)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 40768b4d19..67ab0089fd 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1038,6 +1038,8 @@ static void
Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
On Wed, 12 Aug 2020 at 00:40, Andrew Jones wrote:
> On Tue, Aug 11, 2020 at 10:38:02AM +0800, Haibo Xu wrote:
> > On Mon, 10 Aug 2020 at 19:05, Andrew Jones wrote:
> > >
> > > On Fri, Aug 07, 2020 at 08:10:36AM +, Haibo Xu wrote:
> > > > Add a virtual SPE device for virt machine while using PPI
> > > > 5 for SPE overflow interrupt number.
> > >
> > > Any reason PPI 5 was selected?
> > >
> >
> > No special reason to choose PPI 5. Just re-use the setting in kvmtool.
>
> Please write in the commit message that kvmtool has already selected PPI 5
> for this purpose.
>
Ok, will fix it.
> > > > +fdt_add_spe_nodes(vms);
> > > > +
> > >
> > > You didn't add any compat code, which means all virt machine types are
> now
> > > getting an SPE FDT node, ACPI table change, and, most importantly, PPI
> 5
> > > has gone from unallocated to allocated. We definitely need compat code.
> > >
> >
> > So the 'compat code' here means to only add the SPE node in KVM mode?
>
> No, it means only add it for the 5.2 and later machine types. You'll see
> what I mean when you study the patchset I pointed out, which is also only
> for 5.2 and later machine types.
>
Ok, thanks for the clarification!
> > > > +if (switched_level & KVM_ARM_DEV_SPE) {
> > > > +qemu_set_irq(cpu->spe_interrupt,
> > > > + !!(run->s.regs.device_irq_level &
> KVM_ARM_DEV_SPE));
> > > > +switched_level &= ~KVM_ARM_DEV_SPE;
> > > > +}
> > > > +
> > >
> > > Did you test with a userspace irqchip?
> >
> > No, I just tested with an in-kernel irqchip.
> > Actually, the current kernel vSPE patch doesn't support a userspace
> irqchip.
> > AFAIK, the userspace irqchip support should be ready in the next
> > kernel patch series
> > which will be sent out for review in the middle of September.
>
> It probably doesn't hurt to do the above hunk already, hoping it will just
> work when it's possible to test, but I generally prefer only adding tested
> code. Maybe this hunk should be a separate patch with a commit message
> explaining that it's untested?
>
Good idea! I will drop the hunk in this series, and send out a separate
patch to enable it
once the kernel support is ready!
> Thanks,
> drew
>
>
Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
On Tue, Aug 11, 2020 at 10:38:02AM +0800, Haibo Xu wrote:
> On Mon, 10 Aug 2020 at 19:05, Andrew Jones wrote:
> >
> > On Fri, Aug 07, 2020 at 08:10:36AM +, Haibo Xu wrote:
> > > Add a virtual SPE device for virt machine while using PPI
> > > 5 for SPE overflow interrupt number.
> >
> > Any reason PPI 5 was selected?
> >
>
> No special reason to choose PPI 5. Just re-use the setting in kvmtool.
Please write in the commit message that kvmtool has already selected PPI 5
for this purpose.
> > > +fdt_add_spe_nodes(vms);
> > > +
> >
> > You didn't add any compat code, which means all virt machine types are now
> > getting an SPE FDT node, ACPI table change, and, most importantly, PPI 5
> > has gone from unallocated to allocated. We definitely need compat code.
> >
>
> So the 'compat code' here means to only add the SPE node in KVM mode?
No, it means only add it for the 5.2 and later machine types. You'll see
what I mean when you study the patchset I pointed out, which is also only
for 5.2 and later machine types.
> > > +if (switched_level & KVM_ARM_DEV_SPE) {
> > > +qemu_set_irq(cpu->spe_interrupt,
> > > + !!(run->s.regs.device_irq_level &
> > > KVM_ARM_DEV_SPE));
> > > +switched_level &= ~KVM_ARM_DEV_SPE;
> > > +}
> > > +
> >
> > Did you test with a userspace irqchip?
>
> No, I just tested with an in-kernel irqchip.
> Actually, the current kernel vSPE patch doesn't support a userspace irqchip.
> AFAIK, the userspace irqchip support should be ready in the next
> kernel patch series
> which will be sent out for review in the middle of September.
It probably doesn't hurt to do the above hunk already, hoping it will just
work when it's possible to test, but I generally prefer only adding tested
code. Maybe this hunk should be a separate patch with a commit message
explaining that it's untested?
Thanks,
drew
Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
On Mon, 10 Aug 2020 at 19:05, Andrew Jones wrote:
>
> On Fri, Aug 07, 2020 at 08:10:36AM +, Haibo Xu wrote:
> > Add a virtual SPE device for virt machine while using PPI
> > 5 for SPE overflow interrupt number.
>
> Any reason PPI 5 was selected?
>
No special reason to choose PPI 5. Just re-use the setting in kvmtool.
> >
> > Signed-off-by: Haibo Xu
> > ---
> > hw/arm/virt-acpi-build.c| 3 +++
> > hw/arm/virt.c | 42 +
> > include/hw/acpi/acpi-defs.h | 1 +
> > include/hw/arm/virt.h | 1 +
> > target/arm/cpu.c| 2 ++
> > target/arm/cpu.h| 2 ++
> > target/arm/kvm.c| 6 ++
> > 7 files changed, 57 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 91f0df7b13..5073ba22a5 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> > VirtMachineState *vms)
> > if (arm_feature(>env, ARM_FEATURE_PMU)) {
> > gicc->performance_interrupt =
> > cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> > }
> > +if (arm_feature(>env, ARM_FEATURE_SPE)) {
> > +gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> > +}
> > if (vms->virt) {
> > gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
> > }
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ecfee362a1..c40819705d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState
> > *vms)
> > }
> > }
> >
> > +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> > +{
> > +CPUState *cpu;
> > +ARMCPU *armcpu;
> > +uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> > +
> > +CPU_FOREACH(cpu) {
> > +armcpu = ARM_CPU(cpu);
> > +if (!arm_feature(>env, ARM_FEATURE_SPE)) {
> > +return;
> > +}
> > +if (kvm_enabled()) {
> > +if (kvm_irqchip_in_kernel()) {
> > +kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> > +}
> > +kvm_arm_spe_init(cpu);
> > +}
> > +}
> > +
> > +if (vms->gic_version == VIRT_GIC_VERSION_2) {
> > +irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> > + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> > + (1 << vms->smp_cpus) - 1);
> > +}
> > +
> > +armcpu = ARM_CPU(qemu_get_cpu(0));
> > +qemu_fdt_add_subnode(vms->fdt, "/spe");
> > +if (arm_feature(>env, ARM_FEATURE_V8)) {
> > +const char compat[] = "arm,statistical-profiling-extension-v1";
> > +qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> > + compat, sizeof(compat));
> > +qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> > + GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ,
> > irqflags);
> > +}
> > +}
>
> Please look at [1]. I've refactored fdt_add_pmu_nodes(), so it no longer
> makes a good pattern for this function.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg727588.html
>
>
Ok, I will spend some time studying your patches, and rework the related patches
in the next version.
> > +
> > static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
> > {
> > DeviceState *dev;
> > @@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
> > qdev_get_gpio_in(vms->gic, ppibase
> > + VIRTUAL_PMU_IRQ));
> >
> > +qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> > +qdev_get_gpio_in(vms->gic, ppibase
> > + + VIRTUAL_SPE_IRQ));
> > +
> > sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> > ARM_CPU_IRQ));
> > sysbus_connect_irq(gicbusdev, i + smp_cpus,
> > qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > @@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
> >
> > fdt_add_pmu_nodes(vms);
> >
> > +fdt_add_spe_nodes(vms);
> > +
>
> You didn't add any compat code, which means all virt machine types are now
> getting an SPE FDT node, ACPI table change, and, most importantly, PPI 5
> has gone from unallocated to allocated. We definitely need compat code.
>
So the 'compat code' here means to only add the SPE node in KVM mode?
> > create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
> >
> > if (vms->secure) {
> > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> > index 38a42f409a..56a7f38ae4 100644
> > --- a/include/hw/acpi/acpi-defs.h
> > +++ b/include/hw/acpi/acpi-defs.h
> > @@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
> > uint32_t vgic_interrupt;
> > uint64_t
Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
On Fri, Aug 07, 2020 at 08:10:36AM +, Haibo Xu wrote:
> Add a virtual SPE device for virt machine while using PPI
> 5 for SPE overflow interrupt number.
Any reason PPI 5 was selected?
>
> Signed-off-by: Haibo Xu
> ---
> hw/arm/virt-acpi-build.c| 3 +++
> hw/arm/virt.c | 42 +
> include/hw/acpi/acpi-defs.h | 1 +
> include/hw/arm/virt.h | 1 +
> target/arm/cpu.c| 2 ++
> target/arm/cpu.h| 2 ++
> target/arm/kvm.c| 6 ++
> 7 files changed, 57 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..5073ba22a5 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> if (arm_feature(>env, ARM_FEATURE_PMU)) {
> gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> }
> +if (arm_feature(>env, ARM_FEATURE_SPE)) {
> +gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> +}
> if (vms->virt) {
> gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
> }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ecfee362a1..c40819705d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState
> *vms)
> }
> }
>
> +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> +{
> +CPUState *cpu;
> +ARMCPU *armcpu;
> +uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> +
> +CPU_FOREACH(cpu) {
> +armcpu = ARM_CPU(cpu);
> +if (!arm_feature(>env, ARM_FEATURE_SPE)) {
> +return;
> +}
> +if (kvm_enabled()) {
> +if (kvm_irqchip_in_kernel()) {
> +kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> +}
> +kvm_arm_spe_init(cpu);
> +}
> +}
> +
> +if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> + (1 << vms->smp_cpus) - 1);
> +}
> +
> +armcpu = ARM_CPU(qemu_get_cpu(0));
> +qemu_fdt_add_subnode(vms->fdt, "/spe");
> +if (arm_feature(>env, ARM_FEATURE_V8)) {
> +const char compat[] = "arm,statistical-profiling-extension-v1";
> +qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> + compat, sizeof(compat));
> +qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> + GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ,
> irqflags);
> +}
> +}
Please look at [1]. I've refactored fdt_add_pmu_nodes(), so it no longer
makes a good pattern for this function.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg727588.html
> +
> static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
> {
> DeviceState *dev;
> @@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
> qdev_get_gpio_in(vms->gic, ppibase
> + VIRTUAL_PMU_IRQ));
>
> +qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> +qdev_get_gpio_in(vms->gic, ppibase
> + + VIRTUAL_SPE_IRQ));
> +
> sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> ARM_CPU_IRQ));
> sysbus_connect_irq(gicbusdev, i + smp_cpus,
> qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> @@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
>
> fdt_add_pmu_nodes(vms);
>
> +fdt_add_spe_nodes(vms);
> +
You didn't add any compat code, which means all virt machine types are now
getting an SPE FDT node, ACPI table change, and, most importantly, PPI 5
has gone from unallocated to allocated. We definitely need compat code.
> create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
>
> if (vms->secure) {
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 38a42f409a..56a7f38ae4 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
> uint32_t vgic_interrupt;
> uint64_t gicr_base_address;
> uint64_t arm_mpidr;
> +uint16_t spe_interrupt; /* ACPI 6.3 */
> } QEMU_PACKED;
>
> typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index dff67e1bef..56c83224d2 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -49,6 +49,7 @@
> #define ARCH_TIMER_NS_EL1_IRQ 14
> #define ARCH_TIMER_NS_EL2_IRQ 10
>
> +#define VIRTUAL_SPE_IRQ 5
> #define VIRTUAL_PMU_IRQ 7
>
> #define PPI(irq) ((irq) +
[PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine
Add a virtual SPE device for virt machine while using PPI
5 for SPE overflow interrupt number.
Signed-off-by: Haibo Xu
---
hw/arm/virt-acpi-build.c| 3 +++
hw/arm/virt.c | 42 +
include/hw/acpi/acpi-defs.h | 1 +
include/hw/arm/virt.h | 1 +
target/arm/cpu.c| 2 ++
target/arm/cpu.h| 2 ++
target/arm/kvm.c| 6 ++
7 files changed, 57 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 91f0df7b13..5073ba22a5 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
if (arm_feature(>env, ARM_FEATURE_PMU)) {
gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
}
+if (arm_feature(>env, ARM_FEATURE_SPE)) {
+gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
+}
if (vms->virt) {
gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ecfee362a1..c40819705d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms)
}
}
+static void fdt_add_spe_nodes(const VirtMachineState *vms)
+{
+CPUState *cpu;
+ARMCPU *armcpu;
+uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+
+CPU_FOREACH(cpu) {
+armcpu = ARM_CPU(cpu);
+if (!arm_feature(>env, ARM_FEATURE_SPE)) {
+return;
+}
+if (kvm_enabled()) {
+if (kvm_irqchip_in_kernel()) {
+kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
+}
+kvm_arm_spe_init(cpu);
+}
+}
+
+if (vms->gic_version == VIRT_GIC_VERSION_2) {
+irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+ GIC_FDT_IRQ_PPI_CPU_WIDTH,
+ (1 << vms->smp_cpus) - 1);
+}
+
+armcpu = ARM_CPU(qemu_get_cpu(0));
+qemu_fdt_add_subnode(vms->fdt, "/spe");
+if (arm_feature(>env, ARM_FEATURE_V8)) {
+const char compat[] = "arm,statistical-profiling-extension-v1";
+qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
+ compat, sizeof(compat));
+qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
+ GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ,
irqflags);
+}
+}
+
static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
{
DeviceState *dev;
@@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
qdev_get_gpio_in(vms->gic, ppibase
+ VIRTUAL_PMU_IRQ));
+qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
+qdev_get_gpio_in(vms->gic, ppibase
+ + VIRTUAL_SPE_IRQ));
+
sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
ARM_CPU_IRQ));
sysbus_connect_irq(gicbusdev, i + smp_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
@@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
fdt_add_pmu_nodes(vms);
+fdt_add_spe_nodes(vms);
+
create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
if (vms->secure) {
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 38a42f409a..56a7f38ae4 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
uint32_t vgic_interrupt;
uint64_t gicr_base_address;
uint64_t arm_mpidr;
+uint16_t spe_interrupt; /* ACPI 6.3 */
} QEMU_PACKED;
typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index dff67e1bef..56c83224d2 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -49,6 +49,7 @@
#define ARCH_TIMER_NS_EL1_IRQ 14
#define ARCH_TIMER_NS_EL2_IRQ 10
+#define VIRTUAL_SPE_IRQ 5
#define VIRTUAL_PMU_IRQ 7
#define PPI(irq) ((irq) + 16)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 40768b4d19..67ab0089fd 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1038,6 +1038,8 @@ static void arm_cpu_initfn(Object *obj)
"gicv3-maintenance-interrupt", 1);
qdev_init_gpio_out_named(DEVICE(cpu), >pmu_interrupt,
"pmu-interrupt", 1);
+qdev_init_gpio_out_named(DEVICE(cpu), >spe_interrupt,
+ "spe-interrupt", 1);
#endif
/* DTB consumers generally don't in fact care what the 'compatible'
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index fe0ac14386..4bf8591df8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -790,6 +790,8 @@ struct ARMCPU {
qemu_irq
