Hi Pratyush,

On 19/12/16 07:13, Pratyush Anand wrote:
> Purgatory sha verification is very slow when D-cache is not enabled there.
> We need to enable MMU as well to enable D-Cache.Therefore,we need to an
> identity mapped page table in purgatory.
> 
> Since debugging is very difficult in purgatory therefore we prefer to do as
> much work as possible in kexec.
> 
> This patch prepares page table for purgatory in advance. We support only 4K
> page table,because it will be available on most of the platform. This page
> table is passed to the purgatory as a new segment.
> 
> VA bit is fixed as 48, page table level is 3 where 3rd level page table
> contains 2M block entries.

> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 04fd3968bb52..c2c8ff1b6940 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -24,6 +24,45 @@
>  #include "kexec-syscall.h"
>  #include "arch/options.h"
>  
> +/*
> + * kexec creates identity page table to be used in purgatory so that
> + * dcache verification becomes faster.
> + *
> + * These are the definitions to be used by page table creation routine.
> + *
> + * Only 4K page table, 3 level, 2M block mapping, 48bit VA is supported
> + */
> +#define PGDIR_SHIFT          39
> +#define PUD_SHIFT            30
> +#define PMD_SHIFT            21
> +#define PTRS_PER_PGD         0x1FF
> +#define PTRS_PER_PUD         0x1FF
> +#define PTRS_PER_PMD         0x1FF

Aren't these 0x200 for 4K pages in the kernel? It looks like you use them as a
mask instead.


> +#define PMD_TYPE_TABLE               (3UL << 0)
> +#define PMD_TYPE_SECT                (1UL << 0)
> +#define PMD_SECT_AF          (1UL << 10)
> +#define PMD_ATTRINDX(t)              ((unsigned long)(t) << 2)

> +#define MT_NORMAL            4

This needs to correspond to the part of MAIR that describes the memory type for
'normal'. You define MT_NORMAL again in the next patch, it would be better to
share the definition so they can't be different. (I don't see why you can't use 
0!)


> +#define PMD_FLAGS_NORMAL     (PMD_TYPE_SECT | PMD_SECT_AF)
> +#define MMU_FLAGS_NORMAL     (PMD_ATTRINDX(MT_NORMAL) | PMD_FLAGS_NORMAL)

The SH and AP bits are left as zero, which means non-shareable, read/writeable,
(which I think is fine).


> +#define SECTION_SIZE         (2 * 1024 * 1024)
> +#define PAGE_SIZE            (4 * 1024)
> +/* Since we are using 3 level of page tables, therefore minimum number of
> + * table will be 3. Most likely we will never need more than 3. Each entry
> + * in level 3 page table can map 2MB memory area. Thus a level 3 page table
> + * indexed by bit 29:21 can map a total of 1G memory area. Therefore, if
> + * any segment crosses 1G boundary, then we will need one more level 3
> + * table. Similarly, level 2 page table indexed by bit 38:30 can map a
> + * total of 512G memory area. If segment addresses are more than 512G apart
> + * then we will need two more table for each such block. We do not expect
> + * any memory segment to cross 512G boundary, however if we will ever wish
> + * to support uart debugging in purgatory then that might cross the
> + * boundary and therefore additional 2 more table space. Thus we will need
> + * maximum of 6 table space.

Surely we only need 6 page_size table entries if we support uart debugging,
which your 'if we ever' suggests we don't. So surely we only need 3, and a
comment that this needs expanding to 6 if we need to map two distinct areas.


> + */
> +#define MAX_PGTBLE_SZ        (6 * 4096)
> +static int next_tbl_cnt = 1;
> +
>  /* Global varables the core kexec routines expect. */
>  
>  unsigned char reuse_initrd;
> @@ -316,6 +355,117 @@ unsigned long arm64_locate_kernel_segment(struct 
> kexec_info *info)
>       return hole;
>  }
>  
> +static unsigned long *create_table_entry(unsigned long *pgtbl_buf,
> +             unsigned long pgtbl_mem, unsigned long *tbl,
> +             unsigned long virt, int shift,
> +             unsigned long ptrs)
> +{
> +     unsigned long index, desc, offset;
> +
> +     index = (virt >> shift) & ptrs;
> +     /* check if we have allocated a table already for this index */
> +     if (tbl[index]) {
> +             /*
> +              * table index will have entry as per purgatory page table
> +              * memory. Find out corresponding buffer address of table.
> +              */
> +             desc = tbl[index] & ~3UL;
> +             offset = desc - pgtbl_mem;

wait .. no .. pgtbl_mem is also a guest physical address, so this is fine... its
not obvious at first glance!  You could do with a typedef to make it clear which
addresses are the guests (pgtable_mem) and which are the hosts (pgtable_buf).
Something like phys_addr_t?


> +             return &pgtbl_buf[offset >> 3];
> +     }
> +
> +     /*
> +      * Always write page table content as per page table memory allocated
> +      * for purgaory area, but return corresponding buffer area alloced

Nit: purgatory, allocated,

> +      * in kexec
> +      */
> +     if (next_tbl_cnt > 5)
> +             die("%s: No more memory for page table\n", __func__);

die()? With a bit of juggling can't we return an error so we never try to enable
the MMU+dcache instead?


> +
> +     tbl[index] = (pgtbl_mem + PAGE_SIZE * next_tbl_cnt) | PMD_TYPE_TABLE;
> +
> +     return &pgtbl_buf[(next_tbl_cnt++ * PAGE_SIZE) >> 3];
> +}
> +
> +static void craete_block_entry(unsigned long *tbl, unsigned long flags,
> +             unsigned long phys, unsigned long virt)

Why have separate phys and virt parameters if all this ever does is idmap?


> +{
> +     unsigned long index;
> +     unsigned long desc;
> +
> +     index = (virt >> PMD_SHIFT) & PTRS_PER_PMD;

Copying the pmd_index() macro would make this clearer.


> +     desc = (phys >> PMD_SHIFT) << PMD_SHIFT;

Looks like you wanted a PMD_MASK.


> +     desc |= flags;
> +     tbl[index] = desc;
> +}
> +
> +static void create_identity_entry(unsigned long *pgtbl_buf,
> +             unsigned long pgtbl_mem, unsigned long virt,
> +             unsigned long flags)
> +{
> +     unsigned long *tbl = pgtbl_buf;
> +
> +     tbl = create_table_entry(pgtbl_buf, pgtbl_mem, tbl, virt,
> +                     PGDIR_SHIFT, PTRS_PER_PGD);
> +     tbl = create_table_entry(pgtbl_buf, pgtbl_mem, tbl, virt,
> +                     PUD_SHIFT, PTRS_PER_PUD);
> +     craete_block_entry(tbl, flags, virt, virt);

Nit: create

> +}
> +
> +/**
> + * arm64_create_pgtbl_segment - Create page table segments to be used by
> + * purgatory. Page table will have entries to access memory area of all
> + * those segments which becomes part of sha verification in purgatory.
> + * Additionaly, we also create page table for purgatory segment as well.

Nit: additionally,

> + */
> +
> +static int arm64_create_pgtbl_segment(struct kexec_info *info,
> +             unsigned long hole_min, unsigned long hole_max)
> +{
> +     unsigned long *pgtbl_buf;
> +     int i;
> +     unsigned long mstart, mend, pgtbl_mem;
> +     unsigned long purgatory_base, purgatory_len;
> +
> +     pgtbl_buf = xmalloc(MAX_PGTBLE_SZ);
> +     memset(pgtbl_buf, 0, MAX_PGTBLE_SZ);

> +     pgtbl_mem = add_buffer_phys_virt(info, pgtbl_buf, MAX_PGTBLE_SZ,
> +                     MAX_PGTBLE_SZ, PAGE_SIZE, hole_min, hole_max, 1, 0);

I want to check what this does, but this all looks like it is doing the right 
thing.


> +     for (i = 0; i < info->nr_segments; i++) {
> +             if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
> +                     purgatory_base = (unsigned long)info->segment[i].mem;
> +                     purgatory_len = info->segment[i].memsz;
> +             }
> +             mstart = (unsigned long)info->segment[i].mem;
> +             mend = mstart + info->segment[i].memsz;
> +             mstart &= ~(SECTION_SIZE - 1);
> +             while (mstart < mend) {
> +                     create_identity_entry(pgtbl_buf, pgtbl_mem,
> +                                     mstart, MMU_FLAGS_NORMAL);
> +                     mstart += SECTION_SIZE;
> +             }
> +     }


Thanks,

James



_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to