[Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface

Stefano Stabellini posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/alpine.DEB.2.10.1711061412330.30448@sstabellini-ThinkPad-X260
Test checkpatch failed
Test docker passed
Test ppc passed
Test s390x passed
[Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Posted by Stefano Stabellini 6 years, 5 months ago
When QEMU emulates a GICv3, it needs to advertise the presence of the
system register interface, which is done via id_aa64pfr0.

To do that, and at the same time to avoid advertising the presence of
the system register interface when it is actually not available, set a
boolean property in machvirt_init. Check on the boolean property from
register_cp_regs_for_features and set id_aa64pfr0 accordingly.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9e18b41..369d36b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1401,6 +1401,9 @@ static void machvirt_init(MachineState *machine)
             object_property_set_link(cpuobj, OBJECT(secure_sysmem),
                                      "secure-memory", &error_abort);
         }
+        if (vms->gic_version == 3) {
+            object_property_set_bool(cpuobj, true, "gicv3-sysregs", NULL);
+        }
 
         object_property_set_bool(cpuobj, true, "realized", NULL);
         object_unref(cpuobj);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 88578f3..259cad1 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1690,6 +1690,7 @@ static Property arm_cpu_properties[] = {
     DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
                         mp_affinity, ARM64_AFFINITY_INVALID),
     DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
+    DEFINE_PROP_BOOL("gicv3-sysregs", ARMCPU, gicv3_sysregs, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 89d49cd..0015b37 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -657,6 +657,9 @@ struct ARMCPU {
     /* Should CPU start in PSCI powered-off state? */
     bool start_powered_off;
 
+    /* GICv3 sysregs present */
+    bool gicv3_sysregs;
+
     /* Current power state, access guarded by BQL */
     ARMPSCIState power_state;
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 37af750..6f21900 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4687,7 +4687,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             { .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
               .access = PL1_R, .type = ARM_CP_CONST,
-              .resetvalue = cpu->id_aa64pfr0 },
+              .resetvalue = cpu->gicv3_sysregs ? (cpu->id_aa64pfr0|0x01000000) :
+                  cpu->id_aa64pfr0 },
             { .name = "ID_AA64PFR1_EL1", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 1,
               .access = PL1_R, .type = ARM_CP_CONST,

Re: [Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Posted by no-reply@patchew.org 6 years, 5 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Type: series
Message-id: alpine.DEB.2.10.1711061412330.30448@sstabellini-ThinkPad-X260

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/alpine.DEB.2.10.1711061412330.30448@sstabellini-ThinkPad-X260 -> patchew/alpine.DEB.2.10.1711061412330.30448@sstabellini-ThinkPad-X260
Switched to a new branch 'test'
d3e1cec549 aarch64: advertise the GIC system register interface

=== OUTPUT BEGIN ===
Checking PATCH 1/1: aarch64: advertise the GIC system register interface...
ERROR: spaces required around that '|' (ctx:VxV)
#67: FILE: target/arm/helper.c:4690:
+              .resetvalue = cpu->gicv3_sysregs ? (cpu->id_aa64pfr0|0x01000000) :
                                                                   ^

total: 1 errors, 0 warnings, 34 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Posted by Peter Maydell 6 years, 5 months ago
On 6 November 2017 at 22:16, Stefano Stabellini <sstabellini@kernel.org> wrote:
> When QEMU emulates a GICv3, it needs to advertise the presence of the
> system register interface, which is done via id_aa64pfr0.
>
> To do that, and at the same time to avoid advertising the presence of
> the system register interface when it is actually not available, set a
> boolean property in machvirt_init. Check on the boolean property from
> register_cp_regs_for_features and set id_aa64pfr0 accordingly.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9e18b41..369d36b 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1401,6 +1401,9 @@ static void machvirt_init(MachineState *machine)
>              object_property_set_link(cpuobj, OBJECT(secure_sysmem),
>                                       "secure-memory", &error_abort);
>          }
> +        if (vms->gic_version == 3) {
> +            object_property_set_bool(cpuobj, true, "gicv3-sysregs", NULL);
> +        }
>
>          object_property_set_bool(cpuobj, true, "realized", NULL);
>          object_unref(cpuobj);

I thought about this on the cycle into work this morning, and I
think that rather than require every board that uses gicv3
to set a property on the CPU, we should change the definition
of the id_aa64pfr0 register so that rather than being ARM_CP_CONST
it has a readfn, and then at runtime we can get that readfn to
add in the right bit if env->gicv3state is non-null.

I'll put together a patch this afternoon.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Posted by Stefano Stabellini 6 years, 5 months ago
On Tue, 7 Nov 2017, Peter Maydell wrote:
> On 6 November 2017 at 22:16, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > When QEMU emulates a GICv3, it needs to advertise the presence of the
> > system register interface, which is done via id_aa64pfr0.
> >
> > To do that, and at the same time to avoid advertising the presence of
> > the system register interface when it is actually not available, set a
> > boolean property in machvirt_init. Check on the boolean property from
> > register_cp_regs_for_features and set id_aa64pfr0 accordingly.
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 9e18b41..369d36b 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1401,6 +1401,9 @@ static void machvirt_init(MachineState *machine)
> >              object_property_set_link(cpuobj, OBJECT(secure_sysmem),
> >                                       "secure-memory", &error_abort);
> >          }
> > +        if (vms->gic_version == 3) {
> > +            object_property_set_bool(cpuobj, true, "gicv3-sysregs", NULL);
> > +        }
> >
> >          object_property_set_bool(cpuobj, true, "realized", NULL);
> >          object_unref(cpuobj);
> 
> I thought about this on the cycle into work this morning, and I
> think that rather than require every board that uses gicv3
> to set a property on the CPU, we should change the definition
> of the id_aa64pfr0 register so that rather than being ARM_CP_CONST
> it has a readfn, and then at runtime we can get that readfn to
> add in the right bit if env->gicv3state is non-null.
> 
> I'll put together a patch this afternoon.

Great, please CC me when you do, I'll help you test the patch.

Re: [Qemu-devel] [PATCH v2] aarch64: advertise the GIC system register interface
Posted by Peter Maydell 6 years, 5 months ago
On 7 November 2017 at 17:57, Stefano Stabellini <sstabellini@kernel.org> wrote:
> On Tue, 7 Nov 2017, Peter Maydell wrote:
>> I thought about this on the cycle into work this morning, and I
>> think that rather than require every board that uses gicv3
>> to set a property on the CPU, we should change the definition
>> of the id_aa64pfr0 register so that rather than being ARM_CP_CONST
>> it has a readfn, and then at runtime we can get that readfn to
>> add in the right bit if env->gicv3state is non-null.
>>
>> I'll put together a patch this afternoon.
>
> Great, please CC me when you do, I'll help you test the patch.

http://patchwork.ozlabs.org/patch/835300/ -- should already be
in your inbox somewhere...

thanks
-- PMM