Re: [RFC PATCH 0/3] Initial PECI bus support

[Peter Delevoryas]

On Tue, Sep 13, 2022 at 11:20:57AM -0700, Titus Rwantare wrote:
> On Fri, 9 Sept 2022 at 12:54, Peter Delevoryas <peter@pjd.dev> wrote:
> >
> > On Tue, Sep 06, 2022 at 10:05:49PM +0000, Titus Rwantare wrote:
> ...
> > >
> > > This is something that can also be extended as other parameters arise 
> > > that need
> > > to differ between platforms. So far you can have have different CPUs, 
> > > DIMM counts,
> > > DIMM temperatures here. These fields can also be adjusted at runtime 
> > > through qmp.
> >
> > That looks good to me, seems like the standard way to do it in QEMU.
> >
> > >
> > > A lot of the registers are hard coded, see hw/peci/peci-client.c. I'd 
> > > like to
> > > gauge interest in what potential users would like to be adjustable at 
> > > runtime.
> > > I've not written QEMU models that read config files at runtime, something 
> > > I'd
> > > appreciate guidance on.
> >
> > This part I don't totally understand. I also barely know anything about
> > PECI.
> >
> > Is the register location for things different between CPU generations?
> 
> Some things seem to move between generations and others don't move, someone at
> Intel would know better than I do.
> 
> 
> 
> > If so (and I expect it probably is), why is there only a configuration
> > for Sapphire Rapids, and not for the other ones?
> >
> > Is that because of PECI protocol changes between generations?
> 
> I haven't dug into the other machines because of internal demand, but
> I've found that
> with newer generations, more features get used in addition to existing
> ones. It's
> possible these features existed on older machines.
> 
> 
> 
> > In which case, maybe there needs to be a notion of PECI version
> > somewhere?
> >
> > Also, I don't understand why it would be adjustable at runtime, do we
> > change register locations during execution?
> >
> > I would expect it to be part of the board definition.
> >
> > You could provide a bunch of sample configs for the CPU's that you're
> > testing for, and the board configuration could just select the sample
> > config it is using (corresponding to the CPU model).
> >
> > That's the model I would imagine, but I might be missing some important
> > context here.
> 
> I think it would be nice to have additional registers at runtime, at
> the time of writing,
> I don't know how much of the internal workings of Sapphire Rapids
> Intel is willing to
> share publicly. If users are free to separately define registers, I
> don't then get to
> worry about this. e.g. I'd like to simulate errors from the memory 
> controllers.

Oh ok, yeah I guess making it more dynamic shouldn't really hurt
anything. That sounds ok then.  Also yeah, perhaps keeping the register
definitions separate for privacy concerns is necessary.

Reviewed-by: Peter Delevoryas <peter@pjd.dev>

> 
> 
> Titus