Re: [PATCH v2 1/2] x86/msr: Document I/O-like MSR semantics in /dev/cpu/*/msr driver

[Tim Wiederhake]

On Mon, 2026-06-29 at 08:23 -0700, H. Peter Anvin wrote:
> On June 29, 2026 8:14:12 AM PDT, Tim Wiederhake <twiederh@xxxxxxxxxx>
> wrote:
> > On Fri, 2026-06-26 at 14:42 -0700, H. Peter Anvin wrote:
> > > On 2026-06-26 13:33, Tim Wiederhake wrote:
> > > > 
> > > > This version splits the change in two:
> > > > 
> > > >   1/2 adds a comment explaining why the driver loops over the
> > > > same
> > > >       register, so future readers don't have to rediscover the
> > > > rationale.
> > > >   2/2 removes only the read loop, where repeated access to the
> > > > same
> > > >       register is less obviously useful.
> > > > 
> > > The same applies to the read loop, although the use case(s) are
> > > obviously
> > > different.
> > > 
> > > Either way, you risk breaking working tools for no reason.
> > > 
> > > 	-hpa
> > While looking into this I also noticed that /dev/cpu/*/msr is the
> > only
> > register-addressed character device that doesn't advance ppos on
> > read.
> > /dev/cpu/*/cpuid, /dev/port, and /dev/nvram do. That causes ftell()
> > and
> > lseek(SEEK_CUR, 0) to disagree after an fread(). I don't know how
> > important that is though, and whether that is more of a glibc
> > issue.
> > 
> > Can you please point me to a tool that uses the multiple-read
> > behavior?
> > I want to mention that in the file comment, but turbostat, rdmsr,
> > cpupower, coreboot's inteltool, all read exactly 8 bytes.
> > 
> > Thanks for the review,
> > Tim
> > 
> > 
> 
> /dev/ioport behave(d) that way as well, and was the model. 
> 
> However, you are barking up the wrong tree here: when you want to
> change a 25-year-old API for no technical reason — your only argument
> raised has been aestetics — then the burden of proof is on *you* that
> you won't break anything.

I find that behavior unexpected and would have loved to make it less
surprising, but you're right that I can't guarantee nothing breaks.
I'll resend the comment patch as v3.

Thanks for the /dev/ioport pointer.

Regards,
Tim