Message ID | 1394134385-1727-12-git-send-email-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Suppress the ID_AA64DFR0_EL1 PMUVer field, even if the CPU specific > value claims that it exists. QEMU doesn't currently implement it, > and not advertising it prevents the guest from trying to use it > and getting UNDEFs on unimplemented registers. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > This is arguably a hack, but otherwise Linux tries to prod > half a dozen PMU sysregs. Not really. I think sane self-identification trumps dummy feature advertising. Although there is a consistency argument to be made, as to whether you should also wipe-out any other features advertised by this register and friends (e.g. should TraceVer be set?). Regards, Peter > --- > target-arm/helper.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index c18f1a6..e1672aa 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -1929,7 +1929,12 @@ void register_cp_regs_for_features(ARMCPU *cpu) > { .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, > - .resetvalue = cpu->id_aa64dfr0 }, > + /* We mask out the PMUVer field, beacuse 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 }, > { .name = "ID_AA64DFR1_EL1", .state = ARM_CP_STATE_AA64, > .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1, > .access = PL1_R, .type = ARM_CP_CONST, > -- > 1.9.0 > >
On 17 March 2014 05:13, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> Suppress the ID_AA64DFR0_EL1 PMUVer field, even if the CPU specific >> value claims that it exists. QEMU doesn't currently implement it, >> and not advertising it prevents the guest from trying to use it >> and getting UNDEFs on unimplemented registers. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > >> --- >> This is arguably a hack, but otherwise Linux tries to prod >> half a dozen PMU sysregs. > > Not really. I think sane self-identification trumps dummy feature > advertising. Although there is a consistency argument to be made, as > to whether you should also wipe-out any other features advertised by > this register and friends (e.g. should TraceVer be set?). The lack of consistency is what makes it a hack :-) Generally QEMU takes the approach of "report what the h/w reports even if we don't implement it all"; "report what we provide even if that's not the same values as h/w" would be a different approach, but if we wanted that we'd need to do it consistently. Still I think pragmatism wins out here. thanks -- PMM
On Mon, Mar 17, 2014 at 10:58 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 17 March 2014 05:13, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: >> On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> Suppress the ID_AA64DFR0_EL1 PMUVer field, even if the CPU specific >>> value claims that it exists. QEMU doesn't currently implement it, >>> and not advertising it prevents the guest from trying to use it >>> and getting UNDEFs on unimplemented registers. >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >>> --- >>> This is arguably a hack, but otherwise Linux tries to prod >>> half a dozen PMU sysregs. >> >> Not really. I think sane self-identification trumps dummy feature >> advertising. Although there is a consistency argument to be made, as >> to whether you should also wipe-out any other features advertised by >> this register and friends (e.g. should TraceVer be set?). > > The lack of consistency is what makes it a hack :-) Generally > QEMU takes the approach of "report what the h/w reports even > if we don't implement it all"; "report what we provide even > if that's not the same values as h/w" would be a different > approach, but if we wanted that we'd need to do it consistently. I think there is an argument to decide it case by case .. > Still I think pragmatism wins out here. > In cases where QEMU can validly nop the feature in question (like caches etc.) then faking up to match real HW is cool. But if a guest can take a feature advertisment then if() on it to bang on non-existant hardware causing bus errors or exceptions then I think we should remove the advertisements even if it is a deviation from real hw register state. Supporting a good guest that has self identificaton correct seems more worthwhile than support a guest that somehow requires a feature advertisment without actually using the feature. Regards, Peter > thanks > -- PMM >
diff --git a/target-arm/helper.c b/target-arm/helper.c index c18f1a6..e1672aa 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -1929,7 +1929,12 @@ void register_cp_regs_for_features(ARMCPU *cpu) { .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, - .resetvalue = cpu->id_aa64dfr0 }, + /* We mask out the PMUVer field, beacuse 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 }, { .name = "ID_AA64DFR1_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1, .access = PL1_R, .type = ARM_CP_CONST,
Suppress the ID_AA64DFR0_EL1 PMUVer field, even if the CPU specific value claims that it exists. QEMU doesn't currently implement it, and not advertising it prevents the guest from trying to use it and getting UNDEFs on unimplemented registers. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This is arguably a hack, but otherwise Linux tries to prod half a dozen PMU sysregs. --- target-arm/helper.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)