Skip to content

Commit

Permalink
handle overlapping DMI regions
Browse files Browse the repository at this point in the history 
Signed-off-by: Mark Burton <mburton@quicinc.com>
  • Loading branch information
@markfoodyburton
markfoodyburton authored and Alwalid Salama committed Aug 23, 2024
1 parent 7d56052 commit 7a91352
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 49 deletions.
7 changes: 5 additions & 2 deletions qemu-components/common/include/dmi-manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,12 @@ class QemuInstanceDmiManager
auto existing_it = m_regions.find(start);
if (existing_it != m_regions.end()) {
if (existing_it->second.get_size() != size) {
SCP_FATAL("DMI.Libqbox")("Overlapping DMI regions!");
SCP_INFO("DMI.Libqbox")("Overlapping DMI regions!");
m_root.del_subregion(existing_it->second.get_mut_mr());
m_regions.erase(existing_it);
} else {
return; // Identicle region exists
}
return; // Identicle region exists
}
DmiRegion region = DmiRegion(dmi, 0, m_inst, fd);
m_root.add_subregion(region.get_mut_mr(), region.get_key());
Expand Down
14 changes: 9 additions & 5 deletions qemu-components/common/include/libqemu-cxx/libqemu-cxx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@

#pragma once

// SID this one for you to play with!
// #define UNORD
/*
* it seems like there is a trade off between an unordered and ordered map for the cached iommu translations.
* More experimentation is probably required. For now it looks like the unordered map is better,
* but this is left as a convenience for testing the alternative.
*/
#define USE_UNORD

#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -456,15 +460,15 @@ class IOMMUMemoryRegion : public MemoryRegion
IOMMU_RW = 3,
} IOMMUAccessFlags;

typedef struct {
struct IOMMUTLBEntry {
QemuAddressSpace* target_as;
uint64_t iova;
uint64_t translated_addr;
uint64_t addr_mask;
IOMMUAccessFlags perm;
} IOMMUTLBEntry;
};

#ifdef UNORD
#ifdef USE_UNORD
std::unordered_map<uint64_t, qemu::IOMMUMemoryRegion::IOMMUTLBEntry> m_mapped_te;
#else
std::map<uint64_t, qemu::IOMMUMemoryRegion::IOMMUTLBEntry> m_mapped_te;
Expand Down
89 changes: 47 additions & 42 deletions qemu-components/common/include/ports/initiator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
#include <tlm-extensions/underlying-dmi.h>
#include <tlm_sockets_buswidth.h>

// #define DEBUG_CACHE
/*
* Define this as 1 if you want to enable the cache debug mechanism below.
*/
#define DEBUG_CACHE 0

class QemuInitiatorIface
{
Expand Down Expand Up @@ -140,40 +143,44 @@ class QemuInitiatorSocket
* @param base_addr base address of the iommumr memory region in the address space
* @param addr address to translate
* @param flags QEMU read/write request flags
* @param ind index of translation block.
* @param idx index of translation block.
*
*/
void dmi_translate(qemu::IOMMUMemoryRegion::IOMMUTLBEntry* te, std::shared_ptr<qemu::IOMMUMemoryRegion> iommumr,
uint64_t base_addr, uint64_t addr, qemu::IOMMUMemoryRegion::IOMMUAccessFlags flags, int idx)
{
TlmPayload ltrans;
uint64_t tmp;
#ifdef DEBUG_CACHE
#if DEBUG_CACHE
bool incache = false;
qemu::IOMMUMemoryRegion::IOMMUTLBEntry tmpte;
#endif
#endif // DEBUG_CACHE

/*
* Fast path : check to see if the TE is already cached, if so return it straight away.
*/
{
// Really wish we didn't need the lock here
std::lock_guard<std::mutex> lock(m_mutex);

#ifdef UNORD
#ifdef USE_UNORD
auto m = iommumr->m_mapped_te.find(addr >> iommumr->min_page_sz);
if (m != iommumr->m_mapped_te.end()) {
*te = m->second;
return;
}
#else
#else // USE_UNORD

if (iommumr->m_mapped_te.size() > 0) {
auto it = iommumr->m_mapped_te.upper_bound(addr);
if (it != iommumr->m_mapped_te.begin()) {
it--;
if (it != iommumr->m_mapped_te.end() && (it->first) == (addr & ~it->second.addr_mask)) {
*te = it->second;
#ifdef DEBUG_CACHE
#if DEBUG_CACHE
tmpte = *te;
incache = true;
#endif
#endif // DEBUG_CACHE
SCP_TRACE(())
("FAST translate for 0x{:x} : 0x{:x}->0x{:x} (mask 0x{:x})", addr, te->iova,
te->translated_addr, te->addr_mask);
Expand All @@ -182,9 +189,20 @@ class QemuInitiatorSocket
}
}
}
#endif
#endif // USE_UNORD
}

/*
* Slow path, use DMI to investigate the memory, and see what sort of TE we can set up
*
* There are 3 options
* 1/ a real IOMMU region that should be mapped into the IOMMU address space
* 2/ a 'dmi-able' region which is not an IOMMU (e.g. local memory)
* 3/ a 'non-dmi-able' object (e.g. an MMIO device) - a minimum page size will be used for this.
*
* Enable DEBUG_CACHE to see if the fast path should have been used.
*/

// construct maximal mask.
uint64_t start_msk = (base_addr ^ (base_addr - 1)) >> 1;

Expand Down Expand Up @@ -241,7 +259,7 @@ class QemuInitiatorSocket
("Translate 1-1 passthrough 0x{:x}->0x{:x} (mask 0x{:x})", te->iova, te->translated_addr,
te->addr_mask);
}
#ifdef DEBUG_CACHE
#if DEBUG_CACHE
if (incache) {
SCP_WARN(())("Could have used the cache! {:x}\n", addr);
assert(te->iova == tmpte.iova);
Expand All @@ -250,13 +268,13 @@ class QemuInitiatorSocket
assert(te->translated_addr == tmpte.translated_addr);
assert(te->perm == tmpte.perm);
}
#endif
#endif // DEBUG_CACHE
std::lock_guard<std::mutex> lock(m_mutex);
#ifdef UNORD
#ifdef USE_UNORD
iommumr->m_mapped_te[(addr & ~te->addr_mask) >> iommumr->min_page_sz] = *te;
#else
#else // USE_UNORD
iommumr->m_mapped_te[addr & ~te->addr_mask] = *te;
#endif
#endif // USE_UNORD
SCP_DEBUG(())
("Caching TE at addr 0x{:x} (mask {:x})", addr & ~te->addr_mask, te->addr_mask);

Expand Down Expand Up @@ -315,7 +333,7 @@ class QemuInitiatorSocket
* @note Needs to be called with iothread locked as it will be doing several
* updates and we dont want multiple DMI's
*
* @returns The DMI descriptor for the corresponding DMI region
* @returns The DMI descriptor for the corresponding DMI region - this is used to help construct memory maps only.
*/
tlm::tlm_dmi check_dmi_hint_locked(TlmPayload& trans)
{
Expand Down Expand Up @@ -348,6 +366,16 @@ class QemuInitiatorSocket
return dmi_data;
}

/*
* This is the 'special' case of IOMMU's which require an IOMMU memory region setup
* The IOMMU will be constructed here, but not populated - that will happen in the callback
* There will be a 'pair' of new regions, one to hold non iommu regions within this space,
* the other to hold iommu regions themselves.
*
* In extreme circumstances, if the IOMMU DMI to this region previously failed, we may have
* ended up with a normal DMI region here, which needs removing. We do that here, and then simply
* return and wait for a new access to sort things out.
*/
if (u_dmi.has_dmi(gs::tlm_dmi_ex::dmi_iommu)) {
/* We have an IOMMU request setup an IOMMU region */
SCP_INFO(())("IOMMU DMI available for {:x}", trans.get_address());
Expand All @@ -364,6 +392,9 @@ class QemuInitiatorSocket

qemu::RcuReadLock rcu_read_lock = m_inst.get().rcu_read_lock_new();

/* invalidate any 'old' regions we happen to have mapped previously */
invalidate_single_range(start, start + size);

SCP_INFO(())
("Adding IOMMU for VA 0x{:x} [0x{:x} - 0x{:x}]", trans.get_address(), start, start + size);

Expand All @@ -387,8 +418,6 @@ class QemuInitiatorSocket
}
m_r->m_root->add_subregion(*iommumr, start);

/* invalidate any 'old' regions we happen to have mapped previously */
invalidate_direct_mem_ptr(start, start + iommumr->get_size());
} else {
// Previously when looking up a TE, we failed to get the lock, so the DMI failed, we ended up in a
// limited passthrough. Which causes us to re-arrive here.... but, with a DMI hint. Hopefully next time
Expand All @@ -404,30 +433,6 @@ class QemuInitiatorSocket
uint64_t end_range = itr->first + itr->second->get_size();

invalidate_direct_mem_ptr(start_range, end_range);
#if 0
#ifdef UNORD
start_range >>= iommumr->min_page_sz ;
end_range >>= iommumr->min_page_sz ;
#endif
for (auto m : m_mmio_mrs) {
if (m.first <= start_range && (m.first + m.second->get_size()) >= end_range) {
for (auto it = itr->second->m_mapped_te.begin(); it != itr->second->m_mapped_te.end();) {

#ifdef UNORD
if ((it->first << m.second->min_page_sz) + mr_start >= start_range && (it->first<<m.second->min_page_sz) + mr_start < end_range) {
#else
if (it->first + mr_start >= start_range && it->first + mr_start < end_range) {
if (it->first >= start_range && it->first < end_range) {
#endif
m.second->iommu_unmap(&(it->second));
it = itr->second->m_mapped_te.erase(it);
} else
it++;
}
break;
}
}
#endif
}
return dmi_data;
}
Expand Down Expand Up @@ -824,7 +829,7 @@ class QemuInitiatorSocket
if ((mr_start >= start_range && mr_start <= end_range) ||
(mr_end >= start_range && mr_end <= end_range) || (mr_start < start_range && mr_end > end_range)) {
for (auto it = m.second->m_mapped_te.begin(); it != m.second->m_mapped_te.end();) {
#ifdef UNORD
#ifdef USE_UNORD
if ((it->first << m.second->min_page_sz) + mr_start >= start_range &&
(it->first << m.second->min_page_sz) + mr_start < end_range) {
#else
Expand Down

0 comments on commit 7a91352

Please sign in to comment.