> On March 15, 2016, 11:13 p.m., Andreas Sandberg wrote: > > Thanks for looking into this! > > > > I'd suggest that you implement this as a GICv1 without security extensions. > > According to the architecture spec, the ICDISRn registers should be RAZ/WI > > in that case, so no need to store additional state. > > Bjoern A. Zeeb wrote: > Well the problem seems to be that a pfr(1) returns with & 0x000000f0 > being true, which is how FreeBSD (and probably everyone else) would detect > whether SecExt are present. So I assume we should drop the bits as well from > ArmISA.py id_pfr1 ( id_pfr1 = Param.UInt32(0x00001011, "Processor Feature > Register 1") )? > At least currently assuming they are not there is not an option. > > Side note: FreeBSD code doesn't all do the right thing everywhere either > depending on that flag but for as long as it's there we do and gem5 doesn't. > > Andreas Sandberg wrote: > Good point. We obviously shouldn't set the TZ bits in PFR unless security > extensions have been enabled. I have tasked someone with looking into this > issue and I hope to have a patch shortly. > > Andreas Sandberg wrote: > Ok, I have done some more digging. According to the GICv2 manual: > > * A processor that implements the ARM Virtualization Extensions must > also implement the ARM Security Extensions. > * A GIC that implements the GIC Virtualization Extensions is not > required to implement the GIC Security Extensions. > > My interpretation of this is that is incorrect for an operating system to > assume that the GIC supports security extensions just because the CPU does. > (That doesn't mean that we should do the right thing in ID_PFRx though.) > > We are doing a fairly substantial refresh of the GIC code base at the > moment and we will implement GICD_IGROUPr (ICDISR) as a GICv1 without > security extensions, which would implement a superset of this this patch. I > don't have an ETA yet, but I suspect we can post it externally within a > couple of weeks.
OK, I looked at as much bits as I could find. It seems for the GIC in gem5 the TYPER (not named that way; that's a v1 vs. v2 thing I guess) register does the right thing and not set the Security Extensions bit. I have adjusted the arm(v6/7) and the armv8 code for the GIC (v1/2) on FreeBSD to check for that bit before trying to issue writes for ICDISRn. With rgards to the CPU I can't say much apart from that wasn't an issue so far. I am certain FreeBSD does not have proper checks for the SecExt being there in all places it needs to have them either, but that's not a gem5 issue. I'll discard this one and upstream the OS improvement after it's been reviewed there. Thanks for pointing out the right GIC bits here. - Bjoern A. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3375/#review8090 ----------------------------------------------------------- On March 15, 2016, 4:58 p.m., Bjoern A. Zeeb wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3375/ > ----------------------------------------------------------- > > (Updated March 15, 2016, 4:58 p.m.) > > > Review request for Default and Andreas Sandberg. > > > Repository: gem5 > > > Description > ------- > > arm,dev: add (dummy) ISecR support to the PL390 GIC > > Add (dummy) support for Interrupt Security Registers to allow > software to read/write them even though we do not properly > implement checks yet. This avoids hitting panic()s and > seems to be `good enough' to get certain software running > happily. > > > Diffs > ----- > > src/dev/arm/gic_pl390.hh 2fd64ea0a7cb > src/dev/arm/gic_pl390.cc 2fd64ea0a7cb > > Diff: http://reviews.gem5.org/r/3375/diff/ > > > Testing > ------- > > > Thanks, > > Bjoern A. Zeeb > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
