linux-kernel - RE: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MWHPR11MB188601321993CA43E3058C4D8C539@MWHPR11MB1886.namprd11.prod.outlook.com>
Date:   Tue, 11 May 2021 22:51:40 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>
CC:     Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Li Zefan <lizefan@...wei.com>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        Jonathan Corbet <corbet@....net>,
        "Jean-Philippe Brucker" <jean-philippe@...aro.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "Alex Williamson" <alex.williamson@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Tejun Heo <tj@...nel.org>,
        "cgroups@...r.kernel.org" <cgroups@...r.kernel.org>,
        "Wu, Hao" <hao.wu@...el.com>, David Woodhouse <dwmw2@...radead.org>
Subject: RE: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation
 APIs

> From: Jason Gunthorpe <jgg@...dia.com>
> Sent: Tuesday, May 11, 2021 10:39 PM
> 
> On Tue, May 11, 2021 at 09:10:03AM +0000, Tian, Kevin wrote:
> 
> > 3) SRIOV, ENQCMD (Intel):
> > 	- "PASID global" with host-allocated PASIDs;
> > 	- PASID table managed by host (in HPA space);
> > 	- all RIDs bound to this ioasid_fd use the global pool;
> > 	- however, exposing global PASID into guest breaks migration;
> > 	- hybrid scheme: split local PASID range and global PASID range;
> > 	- force guest to use only local PASID range (through vIOMMU);
> > 	- for ENQCMD, configure CPU to translate local->global;
> > 	- for non-ENQCMD, setup both local/global pasid entries;
> > 	- uAPI for range split and CPU pasid mapping:
> >
> >     // set to "PASID global"
> >     ioctl(ioasid_fd, IOASID_SET_HWID_MODE, IOASID_HWID_GLOBAL);
> >
> >     // split local/global range, applying to all RIDs in this fd
> >     // Example: local [0, 1024), global [1024, max)
> >     // local PASID range is managed by guest and migrated as VM state
> >     // global PASIDs are re-allocated and mapped to local PASIDs post
> migration
> >     ioctl(ioasid_fd, IOASID_HWID_SET_GLOBAL_MIN, 1024);
> 
> I'm still not sold that ranges are the best idea here, it just adds
> more state that has to match during migration. Keeping the
> global/local split per RID seems much cleaner to me

With ENQCMD the PASID is kept in CPU MSR, making it a process
context within the guest. When a guest process is bound to two
devices, the same local PASID must be usable on both devices.
Having per RID split cannot guarantee it.

> 
> This is also why I don't really like having the global/local be global
> to the ioasid either. It would be better to specify global/local as
> part of each VFIO_ATTACH_IOASID so each device is moved to the correct
> allocator.

this was my original thought. But for above reason this has to be
a global enforcement in this ioasid fd.

> 
> > When considering SIOV/mdev there is no change to above uAPI sequence.
> > It's n/a for 1) as SIOV requires PASID table in HPA space, nor does it
> > cause any change to 3) regarding to the split range scheme. The only
> >  conceptual change is in 2), where although it's still "PASID per RID" the
> > PASIDs must be managed by host because the parent driver also allocates
> > PASIDs from per-RID space to mark mdev (RID+PASID). But this difference
> > doesn't change the uAPI flow - just treat user-provisioned PASID as 'virtual'
> > and then allocate a 'real' PASID at IOASID_SET_HWID. Later always use the
> > real one when programming PASID entry (IOASID_BIND_PGTABLE) or
> device
> > PASID register (converted in the mediation path).
> 
> It does need some user visible difference because SIOV/mdev is not
> migratable. Only the kernel can select a PASID, userspace (and hence
> the guest) shouldn't have the option to force a specific PASID as the
> PASID space is shared across the entire RID to all VMs using the mdev.

not migratable only when you choose exposing host-allocated PASID
into guest. However in the entire this proposal we actually virtualize
PASIDs, letting the guest manage its own PASID space in all scenarios
(being SR-IOV or SIOV) though the size of PASID space might be different. 
The PASID chosen by guest may be used as the hw PASID when the 
PASID space is delegated to guest (e.g. SR-IOV in scenario 1), or is 
mapped to a different PASID allocated by guest (e.g. in this mdev 
case or ENQCMD in scenario-3). From uAPI p.o.v the userspace just 
needs to attach its own pasid to ioasid while the kernel will decide 
the real hwid underlyingly (being same or different one). Migration 
only needs cover guest-allocated PASIDs, with all host-side PASIDs 
are hidden from userspace and reconstructed in the new machine 
post migration (following common virtualization practice).

The only exception where we return host-allocated PASID to userspace
in scenario-3 is because Qemu needs such information to update CPU
PASID translation table through KVM. Earlier you suggested that this
must be explicitly done through userspace instead of implicit notification
between ioasid and kvm in kernel.

> 
> I don't see any alternative to telling every part if the PASID is
> going to be used by ENQCMD or not, too many important decisions rest
> on this detail.
> 
> Jason

Thanks
Kevin