On Thu, 11 Jul 2024, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> >Sent: Thursday, July 11, 2024 7:34 AM
> >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>
> >Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx;
> >david.e.box@xxxxxxxxxxxxxxx; Brost, Matthew <matthew.brost@xxxxxxxxx>
> >Subject: Re: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT
> >base adjust
> >
> >On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> >
> >> DVSEC offsets are based on the endpoint BAR. If an endpoint is
> >> not avialable allow the offset information to be adjusted by the
> >
> >available
> >
> >> parent driver.
> >>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
> >> ---
> >> drivers/platform/x86/intel/pmt/class.h | 1 +
> >> drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
> >> drivers/platform/x86/intel/vsec.c | 1 +
> >> include/linux/intel_vsec.h | 2 ++
> >> 4 files changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/intel/pmt/class.h
> >b/drivers/platform/x86/intel/pmt/class.h
> >> index a267ac964423..984cd40ee814 100644
> >> --- a/drivers/platform/x86/intel/pmt/class.h
> >> +++ b/drivers/platform/x86/intel/pmt/class.h
> >> @@ -46,6 +46,7 @@ struct intel_pmt_entry {
> >> void __iomem *base;
> >> struct pmt_callbacks *cb;
> >> unsigned long base_addr;
> >> + s32 base_adjust;
> >> size_t size;
> >> u32 guid;
> >> int devid;
> >> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
> >b/drivers/platform/x86/intel/pmt/telemetry.c
> >> index c9feac859e57..5c44e500e8f6 100644
> >> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> >> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> >> @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct
> >intel_pmt_entry *entry,
> >> header->access_type = TELEM_ACCESS(readl(disc_table));
> >> header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> >> header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> >> + if (entry->base_adjust) {
> >> + u32 new_base = header->base_offset + entry->base_adjust;
> >
> >The code setting ->base_adjust is responsible for avoiding stupid settings
> >that would lead to underflows and overflows?
>
> I would think so. Since this driver is not aware of why the adjust is set,
> it is not clear how it would validate it.
>
> Is this a request for documentation of the usage, or did you have a concern
> that we need to verify the value?
It's just that usually in-kernel interfaces too feature sanity checks but
perhaps you're right and the telemetry side just doesn't know enough to
even make a sanity check on the value. Let's just leave this as is for
now, it can be revisited later if there starts to be many usecases for
this.
--
i.
