Re: [PATCH 06/24] iommu/amd: Map vIOMMU VF and VF Control MMIO BARs
Vasant Hegde <[email protected]>
| Newsgroups | dev.linux.lists.iommu,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Suravee, On 7/27/2026 6:58 PM, Suravee Suthikulpanit wrote: > Enable hardware vIOMMU on an IOMMU by locating its PCI vendor-specific > capability (VSC), reading the VF and VF Control BAR addresses, and > mapping them for host access (256MB VF, 4MB VF Control). > > VF Control covers the first 4K of guest IOMMU MMIO (control registers, > trapped by QEMU). VF MMIO covers the third 4K (virtualized by the > IOMMU). Per-guest bases use the Guest ID from the previous patch. > > Initialize the per-amd_iommu gid_ida here so amd_iommu_gid_alloc() can > run when IOMMUFD creates a vIOMMU instance. Export MMIO map helpers and > call amd_viommu_uninit() from IOMMU teardown. > > Signed-off-by: Vasant Hegde <[email protected]> > Signed-off-by: Suravee Suthikulpanit <[email protected]> > --- > drivers/iommu/amd/amd_iommu.h | 2 + > drivers/iommu/amd/amd_iommu_types.h | 34 ++++++++ > drivers/iommu/amd/amd_viommu.h | 6 ++ > drivers/iommu/amd/init.c | 5 +- > drivers/iommu/amd/viommu.c | 124 ++++++++++++++++++++++++++++ > 5 files changed, 169 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h > index 9f2a1a8a6d3c..044bc9a634a1 100644 > --- a/drivers/iommu/amd/amd_iommu.h > +++ b/drivers/iommu/amd/amd_iommu.h > @@ -28,6 +28,8 @@ void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid); > void iommu_feature_enable(struct amd_iommu *iommu, u8 bit); > void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu, > gfp_t gfp, size_t size); > +u8 __iomem * __init iommu_map_mmio_space(u64 address, u64 end); > +void __init iommu_unmap_mmio_space(struct amd_iommu *iommu); > > #ifdef CONFIG_AMD_IOMMU_DEBUGFS > void amd_iommu_debugfs_setup(void); > diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h > index 00f964d5b149..e88e0bacd1a9 100644 > --- a/drivers/iommu/amd/amd_iommu_types.h > +++ b/drivers/iommu/amd/amd_iommu_types.h > @@ -40,6 +40,12 @@ > #define MMIO_RANGE_OFFSET 0x0c > #define MMIO_MISC_OFFSET 0x10 > > +/* vIOMMU Capability offsets (from IOMMU Capability Header) */ > +#define MMIO_VSC_VF_BAR_LO_OFFSET 0x08 > +#define MMIO_VSC_VF_BAR_HI_OFFSET 0x0c > +#define MMIO_VSC_VF_CNTL_BAR_LO_OFFSET 0x10 > +#define MMIO_VSC_VF_CNTL_BAR_HI_OFFSET 0x14 > + > /* Masks, shifts and macros to parse the device range capability */ > #define MMIO_RANGE_LD_MASK 0xff000000 > #define MMIO_RANGE_FD_MASK 0x00ff0000 > @@ -473,6 +479,20 @@ extern bool amdr_ivrs_remap_support; > #define for_each_ivhd_dte_flags(entry) \ > list_for_each_entry((entry), &amd_ivhd_dev_flags_list, list) > > +/* VIOMMU stuff */ > +#define VIOMMU_VF_MMIO_ENTRY_SIZE 4096 > +#define VIOMMU_VFCTRL_MMIO_ENTRY_SIZE 64 > + > +/* Host ioremap/request_mem_region sizes for VF / VF_CNTL BARs */ > +#define VIOMMU_VF_MMIO_MAP_SIZE 0x10000000UL > +#define VIOMMU_VF_CNTL_MMIO_MAP_SIZE 0x400000UL > + > +#define VIOMMU_VF_MMIO_BASE(iommu, guestId) \ > + (iommu->vf_base + (guestId * VIOMMU_VF_MMIO_ENTRY_SIZE)) > + > +#define VIOMMU_VFCTRL_MMIO_BASE(iommu, guestId) \ > + (iommu->vfctrl_base + (guestId * VIOMMU_VFCTRL_MMIO_ENTRY_SIZE)) > + > struct amd_iommu; > struct iommu_domain; > struct irq_domain; > @@ -686,6 +706,20 @@ struct amd_iommu { > */ > u16 cap_ptr; > > + /* Vendor-Specific Capability (VSC) pointer. */ > + u16 vsc_offset; I think we can remove tracking vsc_offset (see below). > + > + /* > + * VF MMIO base physical address. This is needed to calculate/pass > + * per guest VF MMIO address (3rd 4K of IOMMU MMIO space) > + */ > + u64 vf_base_phys; > + u64 vf_cntl_phys; > + > + /* virtual addresses of vIOMMU VF/VF_CNTL BAR */ > + u8 __iomem *vf_base; > + u8 __iomem *vfctrl_base; > + > /* pci domain of this IOMMU */ > struct amd_iommu_pci_seg *pci_seg; > > diff --git a/drivers/iommu/amd/amd_viommu.h b/drivers/iommu/amd/amd_viommu.h > index f08ab9ef23a9..d0c4fdd00809 100644 .../... > diff --git a/drivers/iommu/amd/viommu.c b/drivers/iommu/amd/viommu.c > index f4b5f96d4785..014ae16bf58b 100644 > --- a/drivers/iommu/amd/viommu.c > +++ b/drivers/iommu/amd/viommu.c > @@ -7,9 +7,15 @@ > #define dev_fmt(fmt) pr_fmt(fmt) > > #include <linux/iommu.h> > +#include <linux/amd-iommu.h> > + > +#include <linux/fs.h> > +#include <linux/cdev.h> > +#include <linux/ioctl.h> > #include <linux/iommufd.h> > #include <linux/amd-iommu.h> > #include <uapi/linux/iommufd.h> > +#include <linux/mem_encrypt.h> > > #include <asm/iommu.h> > #include <asm/set_memory.h> > @@ -18,12 +24,130 @@ > #include "amd_iommu.h" > #include "amd_iommu_types.h" > #include "amd_viommu.h" > +#include "../iommu-pages.h" > + > +LIST_HEAD(viommu_devid_map); > + > +static int viommu_init_pci_vsc(struct amd_iommu *iommu) May be viommu_find_pci_vsc as its not initializing, instead find the offset. Also if we pass vsc_offset to viommu_vf_vfcntl_init() -OR- call this function from viommu_vf_vfcntl_init() then we can skip tracking `vsc_offset` in amd_iommu structure. > +{ > + iommu->vsc_offset = pci_find_capability(iommu->dev, PCI_CAP_ID_VNDR); > + if (!iommu->vsc_offset) > + return -ENODEV; > + > + DUMP_printk("device:%s, vsc offset:%04x\n", > + pci_name(iommu->dev), iommu->vsc_offset); > + return 0; > +} > + > +static void amd_viommu_gid_ida_init(struct amd_iommu *iommu) > +{ > + ida_init(&iommu->gid_ida); > + iommu->gid_ida_inited = true; Once we remove `gid_ida_inited` may be we can call `ida_init` inside amd_viommu_init() itself. > +} > + > +static void amd_viommu_gid_ida_fini(struct amd_iommu *iommu) > +{ > + if (!iommu->gid_ida_inited) > + return; > + > + ida_destroy(&iommu->gid_ida); > + iommu->gid_ida_inited = false; > +} > + > +static void __init amd_viommu_vf_vfcntl_unmap(struct amd_iommu *iommu) > +{ > + if (iommu->vfctrl_base) { > + iounmap(iommu->vfctrl_base); > + iommu->vfctrl_base = NULL; > + } > + if (iommu->vf_cntl_phys) > + release_mem_region(iommu->vf_cntl_phys, VIOMMU_VF_CNTL_MMIO_MAP_SIZE); > + > + if (iommu->vf_base) { > + iounmap(iommu->vf_base); > + iommu->vf_base = NULL; > + } > + if (iommu->vf_base_phys) > + release_mem_region(iommu->vf_base_phys, VIOMMU_VF_MMIO_MAP_SIZE); > +} > + > +void __init amd_viommu_uninit(struct amd_iommu *iommu) > +{ > + amd_viommu_gid_ida_fini(iommu); > + amd_viommu_vf_vfcntl_unmap(iommu); > +} > + > +static int __init viommu_vf_vfcntl_init(struct amd_iommu *iommu) > +{ > + u32 lo, hi; > + u64 vf_phys, vf_cntl_phys; > + > + /* Setting up VF and VF_CNTL MMIOs */ > + pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_BAR_LO_OFFSET, &lo); > + pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_BAR_HI_OFFSET, &hi); > + vf_phys = hi; > + vf_phys = (vf_phys << 32) | lo; > + if (!(vf_phys & 1)) { > + pr_err(FW_BUG "vf_phys disabled\n"); > + return -EINVAL; > + } > + > + pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_CNTL_BAR_LO_OFFSET, &lo); > + pci_read_config_dword(iommu->dev, iommu->vsc_offset + MMIO_VSC_VF_CNTL_BAR_HI_OFFSET, &hi); > + vf_cntl_phys = hi; > + vf_cntl_phys = (vf_cntl_phys << 32) | lo; > + if (!(vf_cntl_phys & 1)) { > + pr_err(FW_BUG "vf_cntl_phys disabled\n"); > + return -EINVAL; > + } > + > + if (!vf_phys || !vf_cntl_phys) { > + pr_err(FW_BUG "AMD-Vi: Unassigned VF resources.\n"); > + return -ENOMEM; > + } Redundant check as previous check confirms both vf_phys and vf_cntl_phys is enabled. If we want to check non-zero address then we have to remove bit zero and add check > + > + /* Mapping 256MB of VF and 4MB of VF_CNTL BARs */ > + vf_phys &= ~1ULL; > + iommu->vf_base = iommu_map_mmio_space(vf_phys, VIOMMU_VF_MMIO_MAP_SIZE); > + if (!iommu->vf_base) { > + pr_err("Can't reserve vf_base\n"); > + return -ENOMEM; > + } > + iommu->vf_base_phys = vf_phys; > + > + vf_cntl_phys &= ~1ULL; > + iommu->vfctrl_base = iommu_map_mmio_space(vf_cntl_phys, VIOMMU_VF_CNTL_MMIO_MAP_SIZE); > + if (!iommu->vfctrl_base) { > + pr_err("Can't reserve vfctrl_base\n"); > + goto err_out; > + } > + iommu->vf_cntl_phys = vf_cntl_phys; > + > + pr_debug("%s: IOMMU device:%s, vf_base:%#llx, vfctrl_base:%#llx\n", better DUMP_printk ? > + __func__, pci_name(iommu->dev), vf_phys, vf_cntl_phys); > + return 0; > +err_out: > + amd_viommu_uninit(iommu); Its odd. We shouldn't call amd_viommu_uninit from here. May be call amd_viommu_vf_vfcntl_unmap() ? -Vasant