Muli Ben-Yehuda wrote:
On Tue, Jun 10, 2008 at 09:26:04AM -0500, Anthony Liguori wrote:
Checking against pfn_valid() isn't enough to differentiate between
RAM and MMIO areas. I think the consensus was that we also need to
check PageReserved(), i.e.,
if (pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn))) ...
When checking the error return of gfn_to_pfn(), you should use
is_error_pfn(). There's no need to differentiate mmio/ram pages in
the code, the goal is just error checking.
I'd have to check the exact semantics of is_error_pfn() to see if it
fits, since strictly speaking what we are doing is not checking
pfn_to_page() for errors. We need to differentiate between gfns which
represent RAM (which needs to be mapped into the VT-d page tables) and
gfns which don't (e.g, slots which represent an MMIO region), which
should not be mapped in the VT-d page tables.
Why? Wouldn't MMIO pages have to be mapped in the VT-d page table in
order to support pass-through? It certainly can't hurt, can it?
At any rate, looking at the code again, the else clause is:
+ printk(KERN_DEBUG "kvm_iommu_map_page:"
+ "invalid pfn=%lx\n", pfn);
+ return 0;
Which looks like error handling to me. I don't think it's at all safe
to assume that a slot is either entirely MMIO or entirely RAM. You
could very easily construct a slot that's a mix of both so if this is an
attempt to skip MMIO slots, it's broken.
Regards,
Anthony Liguori
Cheers,
Muli
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
