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

[Luck, Tony]

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.
> 
> > 
> > Reinette
> 
> -Tony

-Tony