On 11/10/2011 10:05 PM, Nadav Har'El wrote: > 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. No, for two reasons. First, the shadow bits are the result of multiplexing guest and host permissions, for example either the guest of host may write-protect a page. Second, the shadow and guest ptes may be in different formats (ept vs ia32). > I knew we had > a problem when building, not copying, PTEs, and hence the patch to > link_shadow_page). In fact that happens to accidentally work, no? Intermediate ptes are always present/write/user, which translates to read/write/execute in EPT. > 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). Don't optimize for least changes, optimize for best result afterwards. We need a third variant of walk_addr_generic that parses EPT format PTEs. Whether that's best done by writing paging_ept.h or modifying paging_tmpl.h, I don't know. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- 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
