VirtualBox

Changeset 90421 in vbox for trunk/src


Ignore:
Timestamp:
Jul 30, 2021 9:21:29 AM (4 years ago)
Author:
vboxsync
Message:

AMD IOMMU: bugref:9654 Fix IOTLB caching, the DTE cache flags were getting overwritten causing passthrough rather than translation.
Also entries added the IOTLB cache preserves the original page sizes, so the assertion in iommuAmdLookupIoAddrRange had to be tweaked.

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp

    r90257 r90421  
    908908
    909909/**
    910  * Adds or updates the I/O device flags for the given device ID.
     910 * Adds a device-table entry to the cache.
    911911 *
    912912 * @returns VBox status code.
    … …  
    916916 * @param   idDevice    The device ID (bus, device, function).
    917917 * @param   pDte        The device table entry.
    918  * @param   fOrMask     The device flags (usually compound flags) to OR in with the
    919  *                      basic flags, see IOMMU_DTE_CACHE_F_XXX.
    920  */
    921 static int iommuAmdDteCacheAdd(PPDMDEVINS pDevIns, uint16_t idDevice, PCDTE_T pDte, uint16_t fOrMask)
    922 {
    923     Assert(pDte);
    924     Assert(idDevice);
    925 
     918 */
     919static int iommuAmdDteCacheAdd(PPDMDEVINS pDevIns, uint16_t idDevice, PCDTE_T pDte)
     920{
    926921    int rc = VINF_SUCCESS;
    927     uint16_t const fFlags   = iommuAmdGetBasicDevFlags(pDte) | IOMMU_DTE_CACHE_F_PRESENT | fOrMask;
     922    uint16_t const fFlags   = iommuAmdGetBasicDevFlags(pDte) | IOMMU_DTE_CACHE_F_PRESENT;
    928923    uint16_t const idDomain = pDte->n.u16DomainId;
    929924
    … …  
    933928    uint16_t const cDteCache = RT_ELEMENTS(pThis->aDteCache);
    934929    uint16_t idxDte = iommuAmdDteCacheEntryLookup(pThis, idDevice);
    935     if (idxDte < cDteCache)
    936     {
    937         pThis->aDteCache[idxDte].fFlags   = fFlags;
    938         pThis->aDteCache[idxDte].idDomain = idDomain;
    939     }
    940     else if ((idxDte = iommuAmdDteCacheEntryGetUnused(pThis)) < cDteCache)
    941     {
    942         pThis->aDeviceIds[idxDte] = idDevice;
    943         pThis->aDteCache[idxDte].fFlags   = fFlags;
    944         pThis->aDteCache[idxDte].idDomain = idDomain;
    945     }
    946     else
    947         rc = VERR_OUT_OF_RESOURCES;
     930    if (idxDte >= cDteCache)
     931    {
     932        idxDte = iommuAmdDteCacheEntryGetUnused(pThis);
     933        if (idxDte < cDteCache)
     934        {
     935            pThis->aDeviceIds[idxDte] = idDevice;
     936            pThis->aDteCache[idxDte].fFlags   = fFlags;
     937            pThis->aDteCache[idxDte].idDomain = idDomain;
     938        }
     939        else
     940            rc = VERR_OUT_OF_RESOURCES;
     941    }
     942    /* else: A DTE cache entry already exists, do nothing. */
    948943
    949944    IOMMU_CACHE_UNLOCK(pDevIns, pThis);
    … …  
    957952 * @param   pDevIns     The IOMMU instance data.
    958953 * @param   idDevice    The device ID (bus, device, function).
    959  * @param   fFlags      Additional device flags to OR with existing flags, see
     954 * @param   fOrMask     Device flags to add to the existing flags, see
    960955 *                      IOMMU_DTE_CACHE_F_XXX.
    961  */
    962 static void iommuAmdDteCacheAddFlags(PPDMDEVINS pDevIns, uint16_t idDevice, uint16_t fFlags)
     956 * @param   fAndMask    Device flags to remove from the existing flags, see
     957 *                      IOMMU_DTE_CACHE_F_XXX.
     958 */
     959static void iommuAmdDteCacheUpdateFlags(PPDMDEVINS pDevIns, uint16_t idDevice, uint16_t fOrMask, uint16_t fAndMask)
    963960{
    964961    PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
    … …  
    969966    if (   idxDte < cDteCache
    970967        && (pThis->aDteCache[idxDte].fFlags & IOMMU_DTE_CACHE_F_PRESENT))
    971         pThis->aDteCache[idxDte].fFlags |= fFlags;
     968    {
     969        uint16_t const fNewFlags = (pThis->aDteCache[idxDte].fFlags | fOrMask) & ~fAndMask;
     970        pThis->aDteCache[idxDte].fFlags = fNewFlags;
     971    }
    972972
    973973    IOMMU_CACHE_UNLOCK(pDevIns, pThis);
    … …  
    11531153{
    11541154    /* Initialize the IOTLB entry with results of the I/O page walk. */
    1155     pIotlbe->Core.Key   = IOMMU_IOTLB_KEY_MAKE(idDomain, uIova);
    1156     pIotlbe->PageLookup = *pPageLookup;
    1157 
    1158     /* Validate. */
    1159     Assert(pIotlbe->Core.Key != IOMMU_IOTLB_KEY_NIL);
    1160     Assert(!pIotlbe->fEvictPending);
     1155    AVLU64KEY const uKey = IOMMU_IOTLB_KEY_MAKE(idDomain, uIova);
     1156    Assert(uKey != IOMMU_IOTLB_KEY_NIL);
    11611157
    11621158    /* Check if the entry already exists. */
    1163     PIOTLBE pFound = (PIOTLBE)RTAvlU64Get(&pThisR3->TreeIotlbe, pIotlbe->Core.Key);
     1159    PIOTLBE pFound = (PIOTLBE)RTAvlU64Get(&pThisR3->TreeIotlbe, uKey);
    11641160    if (!pFound)
    11651161    {
    11661162        /* Insert the entry into the cache. */
     1163        pIotlbe->Core.Key   = uKey;
     1164        pIotlbe->PageLookup = *pPageLookup;
     1165        Assert(!pIotlbe->fEvictPending);
     1166
    11671167        bool const fInserted = RTAvlU64Insert(&pThisR3->TreeIotlbe, &pIotlbe->Core);
    11681168        Assert(fInserted); NOREF(fInserted);
    … …  
    11741174    {
    11751175        /* Update the existing entry. */
     1176        Assert(pFound->Core.Key == uKey);
    11761177        if (pFound->fEvictPending)
    11771178        {
    … …  
    11791180            STAM_COUNTER_INC(&pThis->StatIotlbeLazyEvictReuse); NOREF(pThis);
    11801181        }
    1181         Assert(pFound->PageLookup.cShift == pPageLookup->cShift);
    1182         pFound->PageLookup.fPerm     = pPageLookup->fPerm;
    1183         pFound->PageLookup.GCPhysSpa = pPageLookup->GCPhysSpa;
     1182        pFound->PageLookup = *pPageLookup;
    11841183    }
    11851184}
    … …  
    14021401
    14031402    IOPAGELOOKUP PageLookup;
    1404     PageLookup.GCPhysSpa = pAddrOut->GCPhysSpa & X86_PAGE_4K_BASE_MASK;;
     1403    PageLookup.GCPhysSpa = pAddrOut->GCPhysSpa & X86_PAGE_4K_BASE_MASK;
    14051404    PageLookup.cShift    = pAddrOut->cShift;
    14061405    PageLookup.fPerm     = pAddrOut->fPerm;
    … …  
    33313330
    33323331#ifdef IOMMU_WITH_DTE_CACHE
    3333 # define IOMMU_DTE_CACHE_SET_PF_RAISED(a_pDevIns, a_DevId)  iommuAmdDteCacheAddFlags((a_pDevIns), (a_DevId), \
    3334                                                                                      IOMMU_DTE_CACHE_F_IO_PAGE_FAULT_RAISED)
     3332# define IOMMU_DTE_CACHE_SET_PF_RAISED(a_pDevIns, a_DevId)  iommuAmdDteCacheUpdateFlags((a_pDevIns), (a_DevId), \
     3333                                                                                        IOMMU_DTE_CACHE_F_IO_PAGE_FAULT_RAISED, \
     3334                                                                                        0 /* fAndMask */)
    33353335#else
    33363336# define IOMMU_DTE_CACHE_SET_PF_RAISED(a_pDevIns, a_DevId)  do { } while (0)
    … …  
    38963896        if (RT_SUCCESS(rc))
    38973897        {
    3898             /* Validate results of the translation. */
     3898            /*
     3899             * Validate results of the translation.
     3900             */
     3901            /* The IOTLB cache preserves the original page sizes even though the IOVAs are split into 4K pages. */
    38993902            Assert(PageLookup.cShift >= X86_PAGE_4K_SHIFT && PageLookup.cShift <= 51);
    3900             Assert(!(PageLookup.GCPhysSpa & X86_GET_PAGE_OFFSET_MASK(PageLookup.cShift)));
     3903            Assert(   pfnIoPageLookup != iommuAmdDteLookupPage
     3904                   || !(PageLookup.GCPhysSpa & X86_GET_PAGE_OFFSET_MASK(PageLookup.cShift)));
    39013905            Assert((PageLookup.fPerm & fPerm) == fPerm);
    39023906
    … …  
    39873991    if (RT_SUCCESS(rc))
    39883992    {
     3993#ifdef IOMMU_WITH_IOTLBE_CACHE
     3994        iommuAmdDteCacheAdd(pDevIns, idDevice, &Dte);
     3995#endif
    39893996        if (Dte.n.u1Valid)
    39903997        {
    … …  
    40364043                    {
    40374044                        /* Update that addresses requires translation (cumulative permissions of DTE and I/O page tables). */
    4038                         iommuAmdDteCacheAdd(pDevIns, Aux.idDevice, &Dte, IOMMU_DTE_CACHE_F_ADDR_TRANSLATE);
     4045                        iommuAmdDteCacheUpdateFlags(pDevIns, idDevice, IOMMU_DTE_CACHE_F_ADDR_TRANSLATE, 0 /* fAndMask */);
    40394046                        /* Update IOTLB for the contiguous range of I/O virtual addresses. */
    40404047                        iommuAmdIotlbAddRange(pDevIns, Aux.idDomain, uIova & X86_PAGE_4K_BASE_MASK, cbContiguous, &AddrOut);
    … …  
    40544061#ifdef IOMMU_WITH_IOTLBE_CACHE
    40554062                    /* Update that addresses permissions of DTE apply (but omit address translation). */
    4056                     iommuAmdDteCacheAdd(pDevIns, idDevice, &Dte, IOMMU_DTE_CACHE_F_IO_PERM);
     4063                    iommuAmdDteCacheUpdateFlags(pDevIns, idDevice, IOMMU_DTE_CACHE_F_IO_PERM, IOMMU_DTE_CACHE_F_ADDR_TRANSLATE);
    40574064#endif
    40584065                }
    … …  
    40844091            GCPhysSpa    = uIova;
    40854092            cbContiguous = cbIova;
    4086 
    4087 #ifdef IOMMU_WITH_IOTLBE_CACHE
    4088             /* Update that addresses don't require translation (nor permission checks) but a DTE is present. */
    4089             iommuAmdDteCacheAdd(pDevIns, idDevice, &Dte, 0 /* fFlags */);
    4090 #endif
    40914093        }
    40924094    }
    … …  
    46014603    {
    46024604#ifdef IOMMU_WITH_IRTE_CACHE
    4603         iommuAmdDteCacheAdd(pDevIns, idDevice, &Dte, 0 /* fFlags */);
     4605        iommuAmdDteCacheAdd(pDevIns, idDevice, &Dte);
    46044606#endif
    46054607        /* If the DTE is not valid, all interrupts are forwarded without remapping. */
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette