Re: [PATCH 0/3] arm64:msr: Add MSR driver

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Dec 03, 2020 at 07:25:43PM +0800, Rongwei Wang wrote:
> 
> 
> > 2020年12月3日 下午4:35，Marc Zyngier <maz@kernel.org> 写道：
> > 
> > On 2020-12-03 05:45, Rongwei Wang wrote:
> >>> 2020年12月1日 下午11:37，Marc Zyngier <maz@kernel.org> 写道：
> >>> On 2020-12-01 14:25, wangrongwei wrote:
> >>>>> 2020年12月1日 下午4:12，Marc Zyngier <maz@kernel.org> 写道：
> >>>>> On 2020-12-01 03:09, wangrongwei wrote:
> >>>>>> Hi
> >>>>>> We have validate this driver in vm and physical machine, and works fine.
> >>>>> But what does "work fine" mean? None of these system registers are supposed
> >>>>> to be accessible from userspace, so please explain *what* you are trying to
> >>>>> do with this, other that introducing security holes and general system
> >>>>> instability?
> >>>> I think I know what you mean. Do you want me to describe how we achieved it?
> >>>> In x86, the different registers can be accessed directly using the
> >>>> rdmsr and wrmsr instructions, but in ARM, since these two instructions
> >>>> are missing, so we modify the code segment during runtime, similar to
> >>>> the principle of static_key.
> >>> [...]
> >>> These are implementation details, none of which answer my question:
> >>> What makes you think this is a good idea? I cannot see any legitimate
> >> In fact, I think this tool useful mainly in the following scenarios:
> >> 	1. performance debug
> >> 	2. Arm-core features test
> >> 	3. Debug-tool for kernel developer
> >> Also, for example, MSR-ARM is needed for chip verification and
> >> system-level functional verification.
> >> A simple example, perf stat can test pmu, but the overflow interrupt
> >> function and forced overflow function of pmu is not covered.
> > 
> > But what does it mean to change random system registers while the kernel
> > itself is using them in parallel? All you are introducing is a bunch of
> > uncontrolled, unexpected, and possibly fatal side effects.
> This problem exists when writing to a register, but it does not exist when reading a register.

Reading registers can also have side-effects. For example the ICC_IAR_*
registers are read-sensitive, and reading those could cause the kernel
to stop receiving timer interrupts or similar. The problem /does/ exist
when reading registers.

I concur with Marc and others that this simply isn't safe, regardless of
read or write, and not something we want upstream in Linux.

For debug and testing, I suggest looking at kvm-unit-tests. That has
some PMU tests already, and is gaining the ability to run bare-metal
around now (and so might be helpful for testing HW). Having improved
tests there would benefit everyone.

Thanks,
Mark.