| Message ID | 20161208170549.8793-3-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/08/2016 11:05 AM, Andrew Jones wrote: > The TCG PMU is barely implemented for ARM and not at all implemented > for AArch64. Let's not bother running the TCG-only tests yet. We'll > likely move them to a new TCG-only unittests.cfg at some point before > re-enabling them too. > > Signed-off-by: Andrew Jones <drjones@redhat.com> It is always safe to disable TCG tests, as proposed in this patch. However I don't think we need it because PMU will behave correctly after your PATCH 1: 1. Under TCG AArch32 mode, get_pmu_version() returns 2. This is acceptable for pmu-tcg-icount-1 and pmu-tcg-icount-256. We should allow the tests to proceed. 2. Under TCG AArch64, get_pmu_version() returns 0. pmu-tcg-icount-1 and pmu-tcg-icount-256 will skip because pmu_probe() returns FALSE. As long as there isn't an error, most people will be OK to see SKIP message. Thanks, -Wei > --- > arm/unittests.cfg | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > index 044d97c9e73d..65f9c4c0b9eb 100644 > --- a/arm/unittests.cfg > +++ b/arm/unittests.cfg > @@ -65,15 +65,15 @@ file = pmu.flat > groups = pmu > > # Test PMU support (TCG) with -icount IPC=1 > -[pmu-tcg-icount-1] > -file = pmu.flat > -extra_params = -icount 0 -append '1' > -groups = pmu > -accel = tcg > +#[pmu-tcg-icount-1] > +#file = pmu.flat > +#extra_params = -icount 0 -append '1' > +#groups = pmu > +#accel = tcg > > # Test PMU support (TCG) with -icount IPC=256 > -[pmu-tcg-icount-256] > -file = pmu.flat > -extra_params = -icount 8 -append '256' > -groups = pmu > -accel = tcg > +#[pmu-tcg-icount-256] > +#file = pmu.flat > +#extra_params = -icount 8 -append '256' > +#groups = pmu > +#accel = tcg > -- 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 Fri, Dec 09, 2016 at 10:51:44AM -0600, Wei Huang wrote: > > > On 12/08/2016 11:05 AM, Andrew Jones wrote: > > The TCG PMU is barely implemented for ARM and not at all implemented > > for AArch64. Let's not bother running the TCG-only tests yet. We'll > > likely move them to a new TCG-only unittests.cfg at some point before > > re-enabling them too. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > It is always safe to disable TCG tests, as proposed in this patch. > However I don't think we need it because PMU will behave correctly after > your PATCH 1: > > 1. Under TCG AArch32 mode, get_pmu_version() returns 2. This is > acceptable for pmu-tcg-icount-1 and pmu-tcg-icount-256. We should allow > the tests to proceed. > 2. Under TCG AArch64, get_pmu_version() returns 0. pmu-tcg-icount-1 and > pmu-tcg-icount-256 will skip because pmu_probe() returns FALSE. As long > as there isn't an error, most people will be OK to see SKIP message. I find it a bit annoying to be running KVM tests and need to mentally filter out results (skips or not) for TCG-only tests. I'd rather leave them off for now. Once we get a tcg-only test running framework in place we can uncomment them. Thanks, drew > > Thanks, > -Wei > > > --- > > arm/unittests.cfg | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > > index 044d97c9e73d..65f9c4c0b9eb 100644 > > --- a/arm/unittests.cfg > > +++ b/arm/unittests.cfg > > @@ -65,15 +65,15 @@ file = pmu.flat > > groups = pmu > > > > # Test PMU support (TCG) with -icount IPC=1 > > -[pmu-tcg-icount-1] > > -file = pmu.flat > > -extra_params = -icount 0 -append '1' > > -groups = pmu > > -accel = tcg > > +#[pmu-tcg-icount-1] > > +#file = pmu.flat > > +#extra_params = -icount 0 -append '1' > > +#groups = pmu > > +#accel = tcg > > > > # Test PMU support (TCG) with -icount IPC=256 > > -[pmu-tcg-icount-256] > > -file = pmu.flat > > -extra_params = -icount 8 -append '256' > > -groups = pmu > > -accel = tcg > > +#[pmu-tcg-icount-256] > > +#file = pmu.flat > > +#extra_params = -icount 8 -append '256' > > +#groups = pmu > > +#accel = tcg > > > -- > 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 -- 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 12/08/2016 12:05 PM, Andrew Jones wrote: > The TCG PMU is barely implemented for ARM and not at all implemented > for AArch64. Let's not bother running the TCG-only tests yet. We'll > likely move them to a new TCG-only unittests.cfg at some point before > re-enabling them too. ID_AA64DFR0_EL1 isn't implemented in TCG, which should be first fixed and then tested. I think and hope we're in agreement on that. However the cycle counter, PMCCNTR, is very much implemented on AArch64 TCG. We've been successfully testing it for months. Thanks, Cov
On 9 December 2016 at 17:22, Christopher Covington <cov@codeaurora.org> wrote: > On 12/08/2016 12:05 PM, Andrew Jones wrote: >> The TCG PMU is barely implemented for ARM and not at all implemented >> for AArch64. Let's not bother running the TCG-only tests yet. We'll >> likely move them to a new TCG-only unittests.cfg at some point before >> re-enabling them too. > > ID_AA64DFR0_EL1 isn't implemented in TCG, which should be first fixed > and then tested. I think and hope we're in agreement on that. Hmm? We have { .name = "ID_AA64DFR0_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0, .access = PL1_R, .type = ARM_CP_CONST, /* We mask out the PMUVer field, because we don't currently * implement the PMU. Not advertising it prevents the guest * from trying to use it and getting UNDEFs on registers we * don't implement. */ .resetvalue = cpu->id_aa64dfr0 & ~0xf00 }, which strongly suggests that we implement it... is that buggy somehow? thanks -- PMM -- 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 Fri, Dec 09, 2016 at 12:22:33PM -0500, Christopher Covington wrote: > On 12/08/2016 12:05 PM, Andrew Jones wrote: > > The TCG PMU is barely implemented for ARM and not at all implemented > > for AArch64. Let's not bother running the TCG-only tests yet. We'll > > likely move them to a new TCG-only unittests.cfg at some point before > > re-enabling them too. > > ID_AA64DFR0_EL1 isn't implemented in TCG, which should be first fixed > and then tested. I think and hope we're in agreement on that. It is implemented in TCG. The v8_idregs table has this { .name = "ID_AA64DFR0_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0, .access = PL1_R, .type = ARM_CP_CONST, /* We mask out the PMUVer field, because we don't currently * implement the PMU. Not advertising it prevents the guest * from trying to use it and getting UNDEFs on registers we * don't implement. */ .resetvalue = cpu->id_aa64dfr0 & ~0xf00 }, Notice the big comment stating PMUVer is zero on purpose. > > However the cycle counter, PMCCNTR, is very much implemented on AArch64 > TCG. We've been successfully testing it for months. It's working, yes, but not because the PMU is fully implemented. Any guest that first probes (correctly) and then uses won't even try. The only probe that succeeds (arm32 ID_DFR0) returns 2, which is invalid for AArch64. Since we're not a real kernel, but a unit testing framework, we can bypass probes when appropriate and just test to see what happens. I'm not sure it's worth it for testing TCG, which states it's not implemented yet though. If this is something you need now, then maybe we should add a command line option to this test that allows the probe to be skipped. Thanks, drew > > Thanks, > Cov > > -- > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm > Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code > Aurora Forum, a Linux Foundation Collaborative Project. > -- > 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 -- 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
diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 044d97c9e73d..65f9c4c0b9eb 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -65,15 +65,15 @@ file = pmu.flat groups = pmu # Test PMU support (TCG) with -icount IPC=1 -[pmu-tcg-icount-1] -file = pmu.flat -extra_params = -icount 0 -append '1' -groups = pmu -accel = tcg +#[pmu-tcg-icount-1] +#file = pmu.flat +#extra_params = -icount 0 -append '1' +#groups = pmu +#accel = tcg # Test PMU support (TCG) with -icount IPC=256 -[pmu-tcg-icount-256] -file = pmu.flat -extra_params = -icount 8 -append '256' -groups = pmu -accel = tcg +#[pmu-tcg-icount-256] +#file = pmu.flat +#extra_params = -icount 8 -append '256' +#groups = pmu +#accel = tcg
The TCG PMU is barely implemented for ARM and not at all implemented for AArch64. Let's not bother running the TCG-only tests yet. We'll likely move them to a new TCG-only unittests.cfg at some point before re-enabling them too. Signed-off-by: Andrew Jones <drjones@redhat.com> --- arm/unittests.cfg | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)