Re: [PATCH v9 01/12] platform/x86/intel/{pmt,vsec}: Prevent unbind via sysfs

[Luck, Tony]

On Fri, Jul 10, 2026 at 10:01:34AM -0700, Luck, Tony wrote:
> On Thu, Jul 09, 2026 at 02:12:58PM -0700, Luck, Tony wrote:
> > On Thu, Jul 09, 2026 at 01:48:12PM -0700, Reinette Chatre wrote:
> > > Hi Tony,
> > > 
> > > On 7/9/26 10:41 AM, Luck, Tony wrote:
> > > > On Wed, Jul 08, 2026 at 03:45:03PM -0700, Reinette Chatre wrote:
> > > >> Hi Tony,
> > > >>
> > > >> On 7/1/26 2:35 PM, Tony Luck wrote:
> ...
> > > 
> > > A simpler solution would be ideal. It is not clear to me that there would be
> > > a page fault though. I looked around at what other drivers do and it seems that
> > > readq() may return all 1s in this scenario? Consider for example, how
> > > IXGBE_FAILED_READ_REG, PCI_ERROR_RESPONSE (note the comments above its definition),
> > > and PCI_POSSIBLE_ERROR are used. Some drivers just straight compare against -1 like
> > > the "if (readl(dev->bar + NVME_REG_CSTS) == -1)" in nvme_pci_enable().
> > > There is also this related snippet in Documentation/PCI/pci.rst: "Most x86 platforms
> > > will allow MMIO reads to master abort (a.k.a. "Soft Fail") and return garbage
> > > (e.g. ~0)." Handling ~0 may be even simpler?
> > 
> > The return of all 1s happens in the case that the underlying h/w has gone,
> > so the PCIe request to read the data is aborted. That's not what happens
> > here. When Linux uses ioremap() to set up a virtual address to access an
> > MMIO range the virtual address is allocated from the same virtual space
> > that vmalloc() uses. When that range is iounmap()'d the page tables are
> > invalidated. So any subsequent access gets a page fault (confirmed by my test).
> > 
> > But ... I ran my patch past an AI review and it pointed out the flaw.  The old
> > virtual address may be reallocated by a subsequent ioremap() or vmalloc()
> > call, at which point AET would begin accessing whatever memory/device was
> > now mapped to the virtual address.
> 
> New scheme. Change intel_pmt_get_regions_by_feature() enumeration to
> pass the physical address of the MMIO region instead of the virtual
> address mapping created by intel_pmt_dev_register().
> 
> Then intel_aet.c can use ioremap() to create an independent virtual
> mapping to the AET register region in MMIO space.
> 
> Then it doesn't matter if some unbind or remove operation tears down
> the virtual mapping. Resctrl can keep accessing the MMIO registers.
> 
> The MMIO space is "on package", so can't be physically removed in a
> hot plug operation. The PCIe device could be disabled, but that would
> only result in the 0xfffffffffffffff return value when trying to read
> the registers.
> 
> There's a flaw in every approach. The problem in this one is if Linux
> reprograms the PCIe device BAR registers to move to a different physical
> address, resctrl would be left using mappings to a stale physical
> address. I don't know how to judge if this is a real problem on
> production servers.

Digging back at my earlier assertion about complexity of handling
unbind from the parent driver of pmt_telemetry, some experimentation
showed that when unbind happens at that level, cleanup includes calling
pmt_telem_remove().

So I plan to add a new exported function from the AET code:

	intel_aet_pmt_notify_remove();

The pmt_telemetry driver will call this to let AET know that all the
MMIO virtual pointers are stale. When this happens reading any of the
AET event files will return -EINVAL (and the user will see "unavailable").

Initial PoC code for this seems to work. I'll clean it up and drop the
patches that set .suppress_bind_attrs = true;

> > 
> > > 
> > > Reinette
> > 
> > -Tony
> 
> -Tony

-Tony