Attached draft patch is supposed to help dealing with tracking cacheability attributes on x86, specifically to prevent page aliases using different cacheability attributes. The problem with the approach of extending the set of page types (by splitting PGT_writable_page) is that a simple update of a pte which changes nothing but _PAGE_PWT or _PAGE_PCD doesn''t work, as the scheme requires the old type (e.g. simple writeable page) to first drop its type reference count to zero, before the new type can be established. While in all previous scenarios this doesn''t present a problem, here it simply seems wrong: If the only use is through the pte being updated, this one reference shouldn''t be considered. As I understand it, racing requests to bump the type reference count could be avoided by clearing PGT_validated while retaining the single reference, but what I don''t have a reasonable idea for is how (and where) to handle the transition while retaining the type reference. Thanks, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 3/4/07 15:00, "Jan Beulich" <jbeulich@novell.com> wrote:> Attached draft patch is supposed to help dealing with tracking cacheability > attributes on x86, specifically to prevent page aliases using different > cacheability attributes.How important is this to get right? The Intel manual warns a bit vaguely about it, but I get the impression that nothing actually breaks in terms of cache coherency if a page is mapped with more than one PAT attribute (very much unlike the situation if CPUs have differing MTRR attributes!). The manual explains that even if a UC attribute is chosen, for example, the processor''s cache will continue to snoop for accesses/updates of that line. This is something I''d be very interested to get a definitive answer on from Intel and AMD. -- Keir> The problem with the approach of extending the > set of page types (by splitting PGT_writable_page) is that a simple update > of a pte which changes nothing but _PAGE_PWT or _PAGE_PCD doesn''t > work, as the scheme requires the old type (e.g. simple writeable page) to > first drop its type reference count to zero, before the new type can be > established. > > While in all previous scenarios this doesn''t present a problem, here it simply > seems wrong: If the only use is through the pte being updated, this one > reference shouldn''t be considered. As I understand it, racing requests to > bump the type reference count could be avoided by clearing PGT_validated > while retaining the single reference, but what I don''t have a reasonable > idea for is how (and where) to handle the transition while retaining the > type reference._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 04.04.07 09:16 >>> >On 3/4/07 15:00, "Jan Beulich" <jbeulich@novell.com> wrote: > >> Attached draft patch is supposed to help dealing with tracking cacheability >> attributes on x86, specifically to prevent page aliases using different >> cacheability attributes. > >How important is this to get right? The Intel manual warns a bit vaguely >about it, but I get the impression that nothing actually breaks in terms of >cache coherency if a page is mapped with more than one PAT attribute (very >much unlike the situation if CPUs have differing MTRR attributes!). The >manual explains that even if a UC attribute is chosen, for example, the >processor''s cache will continue to snoop for accesses/updates of that line.Since use of _PAGE_PCD alone (which is what PAGE_KERNEL_NOCACHE maps to) translates to UC-, this is important, as with the MTRRs for such a region being set to WC the effective memory type is WC, and a WC/WB conflict is what the Intel manual specifically mentions must be avoided. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 4/4/07 08:38, "Jan Beulich" <jbeulich@novell.com> wrote:>>> Attached draft patch is supposed to help dealing with tracking cacheability >>> attributes on x86, specifically to prevent page aliases using different >>> cacheability attributes. >> >> How important is this to get right? The Intel manual warns a bit vaguely >> about it, but I get the impression that nothing actually breaks in terms of >> cache coherency if a page is mapped with more than one PAT attribute (very >> much unlike the situation if CPUs have differing MTRR attributes!). The >> manual explains that even if a UC attribute is chosen, for example, the >> processor''s cache will continue to snoop for accesses/updates of that line. > > Since use of _PAGE_PCD alone (which is what PAGE_KERNEL_NOCACHE maps to) > translates to UC-, this is important, as with the MTRRs for such a region > being set to WC the effective memory type is WC, and a WC/WB conflict is > what the Intel manual specifically mentions must be avoided.My thoughts at the time when I allowed PCD and PWT bits to be set is that we should expect that the MTRRs specify all ordinary RAM (memory marked as E820_RAM by the BIOS) as attribute WB. And that unprivileged guests are only allowed to map ordinary RAM, and combination of PCD/PWT with MTRR.WB is always safe afaics. So we should be okay for ordinary domUs, but we do potentially have a ''hole'' here for driver domains, if they are assigned any memory that has a special MTRR attribute. But actually the problem is worse anyway, as Xen is not really well placed to detect cache-attribute conflicts that occur due to aliasing in the physical address space. Think of the classic Linux AMD AGP GART bug, where pages of DRAM get aliased in the AGP aperture. Ouch! :-) So... I question whether it is worth trying to fix this. I don''t think we can make ourselves watertight security-wise for domains which have access to chipset control registers (e.g., AGP GART). We should already be watertight for most driver domain scenarios where the MTRR attribute is WB/WT/UC. Is it worth aiming to fix the fixable hole of driver domains with MTRR attributes WC or WP? Bear in mind furthermore that the regions that would of interest may not even have page_info structs associated with them (since they are iomem, not normal ram) so we can''t do page-granularity type tracking. If this assumption is broken and we *could* have some normal RAM with MTRR.WC then we are surely screwed anyway as Xen relies on cache coherency and normal memory atomicity properties for its safe and secure execution, and those go out the window with the WC attribute. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser <Keir.Fraser@cl.cam.ac.uk> writes:> On 3/4/07 15:00, "Jan Beulich" <jbeulich@novell.com> wrote: > > > Attached draft patch is supposed to help dealing with tracking cacheability > > attributes on x86, specifically to prevent page aliases using different > > cacheability attributes. > > How important is this to get right?Very. We had very hard to track down problems in the mainline kernel from it. Typically only with 3d drivers though. -Andi _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel