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

[Reinette Chatre]

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:
>>> The resctrl AET code uses intel_pmt_get_regions_by_feature() (exported by the
>>> pmt_telemetry module) to obtain pmt_feature_group instances that are populated
>>> using data from the pmt_discovery module. The returned telemetry_region::addr
>>> values are MMIO mappings created in the telemetry driver's probe path,
>>> and resctrl reads from them while a mount is active.
>>>
>>> However, a user can unbind the intel_pmt driver or its parent intel_vsec
>>> from their devices via sysfs "unbind" attributes. Unbinding either of these
>>> drivers would tear down the MMIO mappings, leaving resctrl with stale pointers
>>> and causing a fault on the next access.
>>>
>>> It is safe for the discovery driver to be unbound. It does all its work at
>>> initial probe time and saves a copy of the features and RMID count.
>>>
>>> Set device_driver::suppress_bind_attrs in each driver so that the "bind"
>>> and "unbind" sysfs attributes are not exposed and the user cannot detach
>>> the driver from its devices while resctrl (or any other in-kernel consumer)
>>> may be using the resources.
>>
>> Unbinding via sysfs does not seem to be the only way in which resources that
>> resctrl depend on could disappear.  The "/sys/bus/pci/devices/.../remove"
>> interface looks to be another way? I am not aware of a way to prevent this since
>> this represents how devices could just be yanked from a system. I wonder if
>> resctrl should instead protect itself against telemetry resources disappearing
>> after mount with another helper that telemetry driver can call when the device
>> is being removed or the module is being unloaded. When telemetry driver informs AET
>> that resources are going away after resctrl is mounted then resctrl/AET can return
>> EIO on event read attempts? resctrl could possibly recover when the device/module
>> re-appears but if that is complicated then user space could just be forced to remount
>> resctrl.
>>
>> With this there is no need to prevent unbind here since it creates a false
>> guarantee to resctrl.
> 
> This sounds like a complexity road that I'd prefer not to travel. The AET
> enumeration is handled with a contract between resctrl and the pmt_telemetry
> module. But the mapping of MMIO space is handled by the parent intel_vsec
> driver. So to get notifications of impending unmap of MMIO space the intel_vsec
> would somehow need to be aware that pmt_telemetry had handed out some pointers.
> 
> The "remove" interface might push this trail up additional levels (to the
> PCIe bus driver level?).
> 
> All this complexity to avoid crashing when system operators poke at sysfs
> files without understanding dependencies.
> 
> I propose a simpler solution. Just have the AET code defensively read from
> the MMIO registers with a new function that turns any page fault due to the
> MMIO space being unmapped into an error return.

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?

Reinette