xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org, Wei.Chen@arm.com,
	Henry.Wang@arm.com, Penny.Zheng@arm.com,
	Bertrand.Marquis@arm.com, Julien Grall <julien.grall@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Julien Grall <jgrall@amazon.com>
Subject: Re: [PATCH RFCv2 15/15] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()
Date: Sat, 15 May 2021 10:25:52 +0100	[thread overview]
Message-ID: <2308478e-527b-a54a-206a-785f80515835@xen.org> (raw)
In-Reply-To: <alpine.DEB.2.21.2105141658510.14426@sstabellini-ThinkPad-T480s>

Hi Stefano,

On 15/05/2021 01:02, Stefano Stabellini wrote:
> On Sun, 25 Apr 2021, Julien Grall wrote:
>> From: Julien Grall <julien.grall@arm.com>
>>
>> Now that map_pages_to_xen() has been extended to support 2MB mappings,
>> we can replace the create_mappings() call by map_pages_to_xen() call.
>>
>> This has the advantage to remove the different between 32-bit and 64-bit
>> code.
>>
>> Lastly remove create_mappings() as there is no more callers.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>
>> ---
>>      Changes in v2:
>>          - New patch
>>
>>      TODO:
>>          - Add support for setting the contiguous bit
>> ---
>>   xen/arch/arm/mm.c | 64 +++++------------------------------------------
>>   1 file changed, 6 insertions(+), 58 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index c49403b687f5..5f8ae029dd6d 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -359,40 +359,6 @@ void clear_fixmap(unsigned map)
>>       BUG_ON(res != 0);
>>   }
>>   
>> -/* Create Xen's mappings of memory.
>> - * Mapping_size must be either 2MB or 32MB.
>> - * Base and virt must be mapping_size aligned.
>> - * Size must be a multiple of mapping_size.
>> - * second must be a contiguous set of second level page tables
>> - * covering the region starting at virt_offset. */
>> -static void __init create_mappings(lpae_t *second,
>> -                                   unsigned long virt_offset,
>> -                                   unsigned long base_mfn,
>> -                                   unsigned long nr_mfns,
>> -                                   unsigned int mapping_size)
>> -{
>> -    unsigned long i, count;
>> -    const unsigned long granularity = mapping_size >> PAGE_SHIFT;
>> -    lpae_t pte, *p;
>> -
>> -    ASSERT((mapping_size == MB(2)) || (mapping_size == MB(32)));
>> -    ASSERT(!((virt_offset >> PAGE_SHIFT) % granularity));
>> -    ASSERT(!(base_mfn % granularity));
>> -    ASSERT(!(nr_mfns % granularity));
>> -
>> -    count = nr_mfns / LPAE_ENTRIES;
>> -    p = second + second_linear_offset(virt_offset);
>> -    pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
>> -    if ( granularity == 16 * LPAE_ENTRIES )
>> -        pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
>> -    for ( i = 0; i < count; i++ )
>> -    {
>> -        write_pte(p + i, pte);
>> -        pte.pt.base += 1 << LPAE_SHIFT;
>> -    }
>> -    flush_xen_tlb_local();
>> -}
>> -
>>   #ifdef CONFIG_DOMAIN_PAGE
>>   void *map_domain_page_global(mfn_t mfn)
>>   {
>> @@ -850,36 +816,18 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>>       unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>>       mfn_t base_mfn;
>>       const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : MB(32);
>> -#ifdef CONFIG_ARM_64
>> -    lpae_t *second, pte;
>> -    unsigned long nr_second;
>> -    mfn_t second_base;
>> -    int i;
>> -#endif
>> +    int rc;
>>   
>>       frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
>>       /* Round up to 2M or 32M boundary, as appropriate. */
>>       frametable_size = ROUNDUP(frametable_size, mapping_size);
>>       base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
>>   
>> -#ifdef CONFIG_ARM_64
>> -    /* Compute the number of second level pages. */
>> -    nr_second = ROUNDUP(frametable_size, FIRST_SIZE) >> FIRST_SHIFT;
>> -    second_base = alloc_boot_pages(nr_second, 1);
>> -    second = mfn_to_virt(second_base);
>> -    for ( i = 0; i < nr_second; i++ )
>> -    {
>> -        clear_page(mfn_to_virt(mfn_add(second_base, i)));
>> -        pte = mfn_to_xen_entry(mfn_add(second_base, i), MT_NORMAL);
>> -        pte.pt.table = 1;
>> -        write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], pte);
>> -    }
>> -    create_mappings(second, 0, mfn_x(base_mfn), frametable_size >> PAGE_SHIFT,
>> -                    mapping_size);
>> -#else
>> -    create_mappings(xen_second, FRAMETABLE_VIRT_START, mfn_x(base_mfn),
>> -                    frametable_size >> PAGE_SHIFT, mapping_size);
>> -#endif
>> +    /* XXX: Handle contiguous bit */
>> +    rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
>> +                          frametable_size >> PAGE_SHIFT, PAGE_HYPERVISOR_RW);
>> +    if ( rc )
>> +        panic("Unable to setup the frametable mappings.\n");
> 
> This is a lot better.
> 
> I take that "XXX: Handle contiguous bit" refers to the lack of
> _PAGE_BLOCK. Why can't we just | _PAGE_BLOCK like in other places?

I forgot to add _PAGE_BLOCK, however this is unrelated to my comment.

Currently, the frametable is mapped using 2MB mapping and setting the 
contiguous bit for each entry if the mapping is 32MB aligned.

_PAGE_BLOCK will only create 2MB mapping but will not set the contiguous 
bit. This will increase the pressure on the TLBs (we would get 16 entry 
rather than 1) if on system where the TLBs can take advantange of it.

So map_pages_to_xen() needs to gain support for contiguous bit. I 
haven't yet looked at it (hence the RFC state).

Cheers,

-- 
Julien Grall


  reply	other threads:[~2021-05-15  9:26 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-25 20:13 [PATCH RFCv2 00/15] xen/arm: mm: Remove open-coding mappings Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 01/15] xen/arm: lpae: Rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS Julien Grall
2021-05-11 22:12   ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 02/15] xen/arm: lpae: Use the generic helpers to defined the Xen PT helpers Julien Grall
2021-05-11 22:26   ` Stefano Stabellini
2021-05-12 14:26     ` Julien Grall
2021-05-12 21:30       ` Stefano Stabellini
2021-05-12 22:16         ` Julien Grall
2021-05-13 22:44           ` Stefano Stabellini
2021-07-03 17:32             ` Julien Grall
2021-07-13 20:53               ` Stefano Stabellini
2021-07-14 17:40                 ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 03/15] xen/arm: p2m: Replace level_{orders, masks} arrays with LEVEL_{ORDER, MASK} Julien Grall
2021-05-11 22:33   ` Stefano Stabellini
2021-05-12 14:39     ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 04/15] xen/arm: mm: Allow other mapping size in xen_pt_update_entry() Julien Grall
2021-05-11 22:42   ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 05/15] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted Julien Grall
2021-05-11 22:51   ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 06/15] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings() Julien Grall
2021-05-11 22:58   ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 07/15] xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen() Julien Grall
2021-05-12 21:41   ` Stefano Stabellini
2021-05-12 22:18     ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 08/15] xen/arm32: mm: Check if the virtual address is shared before updating it Julien Grall
2021-05-12 22:00   ` Stefano Stabellini
2021-05-12 22:23     ` Julien Grall
2021-05-13 22:32       ` Stefano Stabellini
2021-05-13 22:59         ` Julien Grall
2021-05-14  1:04           ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 09/15] xen/arm32: mm: Re-implement setup_xenheap_mappings() using map_pages_to_xen() Julien Grall
2021-05-12 22:07   ` Stefano Stabellini
2021-05-13 17:55     ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 10/15] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap Julien Grall
2021-05-12 22:44   ` Stefano Stabellini
2021-05-13 18:09     ` Julien Grall
2021-05-13 22:27       ` Stefano Stabellini
2021-05-15  8:48         ` Julien Grall
2021-05-18  0:37           ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 11/15] xen/arm: mm: Allow page-table allocation from the boot allocator Julien Grall
2021-05-13 22:58   ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 12/15] xen/arm: add Persistent Map (PMAP) infrastructure Julien Grall
2021-04-26  9:41   ` Xia, Hongyan
2021-07-03 17:57     ` Julien Grall
2021-04-28 21:47   ` Wei Liu
2021-05-14 23:25   ` Stefano Stabellini
2021-05-15  8:54     ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 13/15] xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table() Julien Grall
2021-05-14 23:35   ` Stefano Stabellini
2021-05-15  9:03     ` Julien Grall
2021-04-25 20:13 ` [PATCH RFCv2 14/15] xen/arm: mm: Rework setup_xenheap_mappings() Julien Grall
2021-05-14 23:51   ` Stefano Stabellini
2021-05-15  9:21     ` Julien Grall
2021-05-18  0:50       ` Stefano Stabellini
2021-04-25 20:13 ` [PATCH RFCv2 15/15] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen() Julien Grall
2021-05-15  0:02   ` Stefano Stabellini
2021-05-15  9:25     ` Julien Grall [this message]
2021-05-18  0:51       ` Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2308478e-527b-a54a-206a-785f80515835@xen.org \
    --to=julien@xen.org \
    --cc=Bertrand.Marquis@arm.com \
    --cc=Henry.Wang@arm.com \
    --cc=Penny.Zheng@arm.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=Wei.Chen@arm.com \
    --cc=jgrall@amazon.com \
    --cc=julien.grall@arm.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH RFCv2 15/15] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox