Re: [RFC PATCH 0/5] iommu/arm-smmu-v3: Implement Runtime/System Sleep ops

[Mostafa Saleh <smostafa@google.com>]

On Tue, Mar 25, 2025 at 10:55:12AM -0300, Jason Gunthorpe wrote:
> On Mon, Mar 24, 2025 at 05:53:32PM +0000, Pranjal Shrivastava wrote:
> > On Fri, Mar 21, 2025 at 12:30:34PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Mar 21, 2025 at 02:44:09PM +0000, Pranjal Shrivastava wrote:
> > > 
> > > > However, still there's a worry about the reset value of GBPA.Abort as
> > > > pointed out by Robin earlier. Since the reset value of GBPA.Abort is 
> > > > implementation defined.. there's a chance that after a power cycle the
> > > > SMMU wakes up with GBPA configured to bypass.. in such a case, I don't
> > > > think the kernel should be responsible to ensure security..
> > > 
> > > The kernel should be responsible to operate that HW in a secure
> > > way. If the spec doesn't guarentee security then you will need a
> > > ACPI/DT flag to indicate if specific implementations are secure or
> > > not (ie if the implementation preserves GBPA.Abort).
> > > 
> > > Otherwise we'd have to architect around the insecurity somehow and
> > > prevent the SMMU from unpowering if there is any security sensitive
> > > attachment..
> > 
> > I see.. and what shall we do based on that ACPI/DT flag? 
> > Disable pm-runtime for security reasons? That way, the implementations
> > would get to chose if they *deliberately* want to enable runtime pm
> > despite the security issues. It can also act as an additional switch for
> > the pm feature.
> 
> If this is a real problem I would probably figure out a way to mark
> security sensitive attaches (like untrusted, vfio, etc) and only those
> cases would prevent unpowering the smmu if the HW can't be made
> secure.
> 
> You could also disable PM (or rather the flag would enable PM since we
> have to be backwards compat)

I have been thinking more about this.
For platform devices, we don’t do rpm and VFIO gets a PM reference at
open for the whole period of the fd.

For PCI, I see it provides IOCTLs which interacts with RPM
VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY/EXIT

Ideally, linux RPM would handle ordering due to consumer/supplier links,
but in case both the SMMU and the device share the same power domain,
there is this window of time where a malicious userspace can program the
device while there is an unclear SMMU state.

I agree, I don’t like the approach of adding a (yet another) flag for
firmware to populate, I guess we can either:
- Prevent SMMUv3 suspend, and that’s already the status quo as it
  doesn’t support RPM, instead of doing that from the driver by
  detecting non-secure attaches, we can just make VFIO takes a PM
  reference on the IOMMU.

- Disable the PCI device, or unmap it from userspace from VFIO suspend
  handler, and retrieve it’s state on the VFIO resume handler which would
  be ordered after the SMMUv3.

I don't know much about VFIO-pci(and very little about PCI), but I wonder
what happens if userspace powerdown the device and issued a read/write system
call, wouldn’t that cause the kernel to crash? It seems we need to prevent
userspace from interacting with the device while powered off anyway.

Thanks,
Mostafa

> 
> Jason