On Thu, Nov 10, 2011 at 11:45:06AM +1100, Jonathan Gray wrote: > > > On 09.11.2011 14:40, Avi Kivity wrote: > > > > Actually, it looks like an OpenBSD bug. According to the AMD > > > > documentation: > > > >> "The current P-state value can be read using the P-State Status > > > >> Register. The P-State Current Limit Register and the P-State > > > >> Status Register are read-only registers. Writes to these > > > >> registers cause a #GP exception. Support for hardware P-state > > > >> control is indicated by EDX bit 7 as returned by CPUID function > > > >> 8000_0007h. Figure 18-1 shows the format of the P-State Current > > > >> Limit register." > > ... > As pointed out earlier the AMD documentation says it should be, > the MSRs shouldn't be touched if the cpuid flag isn't set.
I wouldn't expect reads to have any effect, with the proposed patch it won't even perform a read. If ACPI indicates that speed scaling is possible, then it should also be safe to assume that reading/writing to the pstate msr's is safe. > Access to unimplemented MSRs often leads to things like faults/reboots > on real hardware. And while a K1x machine that doesn't have hardware > pstate control might not exist now, code that follows AMD's documentation > would work on such a machine. I guess, but, unlike the AMD K7/8 code, ACPI support is required. The driver does not touch the msr's without it. The patch I suggest only increases its reliance on accurate ACPI tables. If this new AMD machine has ACPI _PSS objects but isn't compatible with AMD K1x-style msr's. A new a driver will be required. > The k1x_init() function should probably be renamed to something > like k1x_pstate_init() for clarity as well. Yeah, that's fine... and if you want to do the cpuid check, that's also okay. I'm sorry, but these impossible machines have amd64 processors sitting behind the Intel 440. This configuration has never existed, either. Thanks, -Bryan.
