Re: [Xen-devel] [PATCH] x86/boot: Don't explicitly map the VGA region as UC-
On 05.09.2019 21:04, Andrew Cooper wrote: > All 64-bit capable processors use PAT, and with PAT, it is explicitly > permitted to have mappings with a cacheability different to MTRRs. > > On a native system with a real legacy VGA region, MTRRs will cause the region > to be UC-. Minor correction: MTRRs can't be used to specify UC-. UC- is used in PAT to allow WC from MTRRs to be retained, rather than becoming UC. And hence sensible BIOSes would make this range WC in the MTRRs, not UC. The main question here is whether we can rely on firmware to actually set MTRRs correctly. > When booting virtualised, this range may be RAM and not a legacy > VGA region, at which point it wants to be WB. To limit the effect of improper MTRR settings, did you consider using WT instead? Furthermore, did you consider dynamically changing the involved PTEs depending on whether we run virtualized ourselves? > Use WB mapping in the pagetables, such that in systems without a legacy VGA > region, the RAM between 0xa and 0xc doesn't become unnecesserily UC-. May I suggest s/RAM/range/ ? There's no guarantee that when there's no VGA there, that it would be RAM instead. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: Don't explicitly map the VGA region as UC-
On 06/09/2019 12:06, Roger Pau Monné wrote: > On Thu, Sep 05, 2019 at 08:04:18PM +0100, Andrew Cooper wrote: >> All 64-bit capable processors use PAT, and with PAT, it is explicitly >> permitted to have mappings with a cacheability different to MTRRs. >> >> On a native system with a real legacy VGA region, MTRRs will cause the region >> to be UC-. When booting virtualised, this range may be RAM and not a legacy >> VGA region, at which point it wants to be WB. >> >> Use WB mapping in the pagetables, such that in systems without a legacy VGA >> region, the RAM between 0xa and 0xc doesn't become unnecesserily UC-. >> However, we still must use small pages to avoid the undefined behaviour >> caused >> by superpages crossing MTRRs of different cacheability. >> >> While adjusting the pagetable construction, switch from pfn to idx for >> consistency with all the other construction logic. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Roger Pau Monné > >> --- >> CC: Jan Beulich >> CC: Wei Liu >> CC: Roger Pau Monné >> >> As a future optimisation, Xen could rewrite l2_identmap with a superpage if >> it >> finds that MTRRs are disabled. However, this probably needs to wait until >> Xen >> no longer unconditionally assumes a legacy VGA region to be present in the >> e820 if it finds something other than a hole > Is that still the case? I remember dealing with it when working on the > shim, and I thought the code to unconditionally set a hole in > 0xa-0xc was removed. copy_e820_map() still has the broken logic. I don't recall exactly what state the proposed fixes were in, and I certainly don't have time to dust them off right now. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: Don't explicitly map the VGA region as UC-
On Thu, Sep 05, 2019 at 08:04:18PM +0100, Andrew Cooper wrote: > All 64-bit capable processors use PAT, and with PAT, it is explicitly > permitted to have mappings with a cacheability different to MTRRs. > > On a native system with a real legacy VGA region, MTRRs will cause the region > to be UC-. When booting virtualised, this range may be RAM and not a legacy > VGA region, at which point it wants to be WB. > > Use WB mapping in the pagetables, such that in systems without a legacy VGA > region, the RAM between 0xa and 0xc doesn't become unnecesserily UC-. > However, we still must use small pages to avoid the undefined behaviour caused > by superpages crossing MTRRs of different cacheability. > > While adjusting the pagetable construction, switch from pfn to idx for > consistency with all the other construction logic. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné > --- > CC: Jan Beulich > CC: Wei Liu > CC: Roger Pau Monné > > As a future optimisation, Xen could rewrite l2_identmap with a superpage if it > finds that MTRRs are disabled. However, this probably needs to wait until Xen > no longer unconditionally assumes a legacy VGA region to be present in the > e820 if it finds something other than a hole Is that still the case? I remember dealing with it when working on the shim, and I thought the code to unconditionally set a hole in 0xa-0xc was removed. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/boot: Don't explicitly map the VGA region as UC-
All 64-bit capable processors use PAT, and with PAT, it is explicitly permitted to have mappings with a cacheability different to MTRRs. On a native system with a real legacy VGA region, MTRRs will cause the region to be UC-. When booting virtualised, this range may be RAM and not a legacy VGA region, at which point it wants to be WB. Use WB mapping in the pagetables, such that in systems without a legacy VGA region, the RAM between 0xa and 0xc doesn't become unnecesserily UC-. However, we still must use small pages to avoid the undefined behaviour caused by superpages crossing MTRRs of different cacheability. While adjusting the pagetable construction, switch from pfn to idx for consistency with all the other construction logic. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné As a future optimisation, Xen could rewrite l2_identmap with a superpage if it finds that MTRRs are disabled. However, this probably needs to wait until Xen no longer unconditionally assumes a legacy VGA region to be present in the e820 if it finds something other than a hole --- xen/arch/x86/boot/x86_64.S | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index 5ab24d73fc..1ca986f882 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -51,19 +51,13 @@ GLOBAL(stack_start) /* * Mapping of first 2 megabytes of memory. This is mapped with 4kB mappings * to avoid type conflicts with fixed-range MTRRs covering the lowest megabyte - * of physical memory. In any case the VGA hole should be mapped with type UC. - * Uses 1x 4k page. + * of physical memory. Uses 1x 4k page. */ l1_identmap: -pfn = 0 +idx = 0 .rept L1_PAGETABLE_ENTRIES -/* VGA hole (0xa-0xc) should be mapped UC-. */ -.if pfn >= 0xa0 && pfn < 0xc0 -.quad (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_UCMINUS | MAP_SMALL_PAGES -.else -.quad (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES -.endif -pfn = pfn + 1 +.quad (idx << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES +idx = idx + 1 .endr .size l1_identmap, . - l1_identmap -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
