On Thu, Nov 10, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT": > This is all correct, but the code in question parses the EPT12 table > using the ia32 page table format. They're sufficiently similar so that > it works, but it isn't correct. > > Bit 0: EPT readable, ia32 present > Bit 1: Writable; ia32 meaning dependent on cr0.wp > Bit 2: EPT executable, ia32 user (so, this implementation will interpret > a non-executable EPT mapping, if someone could find a use for it, as a > L2 kernel only mapping) >.... This is a very good point. I was under the mistaken (?) impression that the page-table shadowing code will just copy these bits as-is from the shadowed table (EPT12) to the shadow table (EPT02), without caring what they actually mean. I knew we had a problem when building, not copying, PTEs, and hence the patch to link_shadow_page). Also I realized we sometimes need to actually walk the TDP EPT12+cr3 (e.g., to see if an EPT violation is L1's fault), but I thought this was just the normal TDP walk, which already knows how to correctly read the EPT table. > walk_addr() will also write to bits 6/7, which the L1 won't expect. I didn't notice this :( Back to the drawing board, I guess. I need to figure out exactly what needs to be fixed, and how to do this with the least obtrusive changes to the existing use case (normal shadow page tables, and nested EPT). -- Nadav Har'El | Thursday, Nov 10 2011, nyh@xxxxxxxxxxxxxxxxxxx |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |Learn from mistakes of others; you won't http://nadav.harel.org.il |live long enough to make them all yourself -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
