Re: [Xen-devel] [PATCH v2 2/8] xen/x86: Improvements to build-time pagetable generation

["Jan Beulich" <JBeulich@xxxxxxxx>]

 >>> On 23.02.16 at 17:31, <andrew.cooper3@xxxxxxxxxx> wrote:
> * Additional comments, including size and runtime use.
>  * Consistent use of idx, rather than a mix including pfn.

I don't see anything wrong with this - when we have a PFN why
should we not call it a PFN? As opposed to when we have a table
index not representing the corresponding PFN.

>  GLOBAL(l1_identmap)
> -        pfn = 0
> +        idx = 0
>          .rept L1_PAGETABLE_ENTRIES
>          /* VGA hole (0xa0000-0xc0000) should be mapped UC. */
> -        .if pfn >= 0xa0 && pfn < 0xc0
> -        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR_NOCACHE | MAP_SMALL_PAGES
> +        .if idx >= 0xa0 && idx < 0xc0
> +        .quad (idx << PAGE_SHIFT) | PAGE_HYPERVISOR_NOCACHE
>          .else
> -        .long (pfn << PAGE_SHIFT) | PAGE_HYPERVISOR | MAP_SMALL_PAGES
> +        .quad (idx << PAGE_SHIFT) | PAGE_HYPERVISOR

Please don't eliminate the MAP_SMALL_PAGES here, they serve an
(at least documentation) purpose.

> -/* Mapping of first 16 megabytes of memory. */
> +/*
> + * Space for mapping the first 4GB of memory, with the first 16 megabytes
> + * actualy mapped (mostly using superpages).  Uses 4x 4k pages.
> + */
>  GLOBAL(l2_identmap)
> -        .quad sym_phys(l1_identmap) + __PAGE_HYPERVISOR
> -        pfn = 0
> +        .quad sym_phys(l1_identmap) + PAGE_HYPERVISOR

This is wrong - the G bit is defined for leaf entries only.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel