On 2017年01月22日 17:36, Peter Xu wrote:
Besides this one, I tried to fix the comments in this function as
below, hope this is better (I removed 1-3 thing since I think that's
clearer from below code):
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index e958f53..f3fe8c4 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -735,15 +735,6 @@ static int vtd_page_walk_level(dma_addr_t addr, uint64_t
start,
offset = vtd_iova_level_offset(iova, level);
slpte = vtd_get_slpte(addr, offset);
- /*
- * When one of the following case happens, we assume the whole
- * range is invalid:
- *
- * 1. read block failed
- * 3. reserved area non-zero
- * 2. both read & write flag are not set
- */
-
if (slpte == (uint64_t)-1) {
trace_vtd_page_walk_skip_read(iova, iova_next);
skipped_local++;
@@ -761,20 +752,16 @@ static int vtd_page_walk_level(dma_addr_t addr, uint64_t
start,
write_cur = write && (slpte & VTD_SL_W);
/*
- * As long as we have either read/write permission, this is
- * a valid entry. The rule works for both page or page tables.
+ * As long as we have either read/write permission, this is a
+ * valid entry. The rule works for both page entries and page
+ * table entries.
*/
entry_valid = read_cur | write_cur;
if (vtd_is_last_slpte(slpte, level)) {
entry.target_as = &address_space_memory;
entry.iova = iova & subpage_mask;
- /*
- * This might be meaningless addr if (!read_cur &&
- * !write_cur), but after all this field will be
- * meaningless in that case, so let's share the code to
- * generate the IOTLBs no matter it's an MAP or UNMAP
- */
+ /* NOTE: this is only meaningful if entry_valid == true */
entry.translated_addr = vtd_get_slpte_addr(slpte);
entry.addr_mask = ~subpage_mask;
entry.perm = IOMMU_ACCESS_FLAG(read_cur, write_cur);
Thanks,
-- peterx
I still prefer to do this on patch 18 probably.
Thanks
