| Message ID | 1631652245-30746-1-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,V2] xen/gnttab: Store frame GFN in struct page_info on Arm | expand |
On 14.09.2021 22:44, Oleksandr Tyshchenko wrote: > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1376,14 +1376,18 @@ unsigned long domain_get_maximum_gpfn(struct domain *d) > void share_xen_page_with_guest(struct page_info *page, struct domain *d, > enum XENSHARE_flags flags) > { > + unsigned long type_info; > + > if ( page_get_owner(page) == d ) > return; > > spin_lock(&d->page_alloc_lock); > > /* The incremented type count pins as writable or read-only. */ > - page->u.inuse.type_info = > - (flags == SHARE_ro ? PGT_none : PGT_writable_page) | 1; > + type_info = page->u.inuse.type_info & ~(PGT_type_mask | PGT_count_mask); > + page->u.inuse.type_info = type_info | > + (flags == SHARE_ro ? PGT_none : PGT_writable_page) | > + (1UL << PGT_count_base); Just as a note: If this was x86 code, I'd request the redundant PGT_count_base to be dropped. Constructs like the above is what we have MASK_INSR() for. > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -2204,7 +2204,10 @@ void *alloc_xenheap_pages(unsigned int order, unsigned int memflags) > return NULL; > > for ( i = 0; i < (1u << order); i++ ) > + { > pg[i].count_info |= PGC_xen_heap; > + arch_alloc_xenheap_page(&pg[i]); > + } > > return page_to_virt(pg); > } > @@ -2222,7 +2225,10 @@ void free_xenheap_pages(void *v, unsigned int order) > pg = virt_to_page(v); > > for ( i = 0; i < (1u << order); i++ ) > + { > pg[i].count_info &= ~PGC_xen_heap; > + arch_free_xenheap_page(&pg[i]); > + } > > free_heap_pages(pg, order, true); > } You look to only be adjusting the !SEPARATE_XENHEAP instances of the functions. Isn't 32-bit Arm using SEPARATE_XENHEAP? > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -98,9 +98,18 @@ struct page_info > #define PGT_writable_page PG_mask(1, 1) /* has writable mappings? */ > #define PGT_type_mask PG_mask(1, 1) /* Bits 31 or 63. */ > > - /* Count of uses of this frame as its current type. */ > -#define PGT_count_width PG_shift(2) > -#define PGT_count_mask ((1UL<<PGT_count_width)-1) > + /* 3-bit count of uses of this frame as its current type. */ > +#define PGT_count_base PG_shift(4) > +#define PGT_count_mask PG_mask(7, 4) > + > +/* > + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table frame. I don't know enough Arm details to tell whether this is properly one bit more than the maximum number of physical address bits. Without the extra bit you wouldn't be able to tell apart a guest specified GFN matching the value of PGT_INVALID_FRAME_GFN from an entry which was set from INVALID_GFN. > + * This only valid for the xenheap pages. > + */ > +#define PGT_gfn_width PG_shift(4) > +#define PGT_gfn_mask ((1UL<<PGT_gfn_width)-1) Any reason you don't use PG_mask() here? Any open-coding is prone to people later making mistakes. Jan
On 15.09.21 13:06, Jan Beulich wrote: Hi Jan > On 14.09.2021 22:44, Oleksandr Tyshchenko wrote: >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -1376,14 +1376,18 @@ unsigned long domain_get_maximum_gpfn(struct domain *d) >> void share_xen_page_with_guest(struct page_info *page, struct domain *d, >> enum XENSHARE_flags flags) >> { >> + unsigned long type_info; >> + >> if ( page_get_owner(page) == d ) >> return; >> >> spin_lock(&d->page_alloc_lock); >> >> /* The incremented type count pins as writable or read-only. */ >> - page->u.inuse.type_info = >> - (flags == SHARE_ro ? PGT_none : PGT_writable_page) | 1; >> + type_info = page->u.inuse.type_info & ~(PGT_type_mask | PGT_count_mask); >> + page->u.inuse.type_info = type_info | >> + (flags == SHARE_ro ? PGT_none : PGT_writable_page) | >> + (1UL << PGT_count_base); > Just as a note: If this was x86 code, I'd request the redundant > PGT_count_base to be dropped. Constructs like the above is what we > have MASK_INSR() for. I got it, I will look at it. > >> --- a/xen/common/page_alloc.c >> +++ b/xen/common/page_alloc.c >> @@ -2204,7 +2204,10 @@ void *alloc_xenheap_pages(unsigned int order, unsigned int memflags) >> return NULL; >> >> for ( i = 0; i < (1u << order); i++ ) >> + { >> pg[i].count_info |= PGC_xen_heap; >> + arch_alloc_xenheap_page(&pg[i]); >> + } >> >> return page_to_virt(pg); >> } >> @@ -2222,7 +2225,10 @@ void free_xenheap_pages(void *v, unsigned int order) >> pg = virt_to_page(v); >> >> for ( i = 0; i < (1u << order); i++ ) >> + { >> pg[i].count_info &= ~PGC_xen_heap; >> + arch_free_xenheap_page(&pg[i]); >> + } >> >> free_heap_pages(pg, order, true); >> } > You look to only be adjusting the !SEPARATE_XENHEAP instances of the > functions. Isn't 32-bit Arm using SEPARATE_XENHEAP? Hmm, looks like yes, thank you. At least config.h defines that on Arm32. I will update the instances. > >> --- a/xen/include/asm-arm/mm.h >> +++ b/xen/include/asm-arm/mm.h >> @@ -98,9 +98,18 @@ struct page_info >> #define PGT_writable_page PG_mask(1, 1) /* has writable mappings? */ >> #define PGT_type_mask PG_mask(1, 1) /* Bits 31 or 63. */ >> >> - /* Count of uses of this frame as its current type. */ >> -#define PGT_count_width PG_shift(2) >> -#define PGT_count_mask ((1UL<<PGT_count_width)-1) >> + /* 3-bit count of uses of this frame as its current type. */ >> +#define PGT_count_base PG_shift(4) >> +#define PGT_count_mask PG_mask(7, 4) >> + >> +/* >> + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table frame. > I don't know enough Arm details to tell whether this is properly > one bit more than the maximum number of physical address bits. > Without the extra bit you wouldn't be able to tell apart a > guest specified GFN matching the value of PGT_INVALID_FRAME_GFN > from an entry which was set from INVALID_GFN. Really good point. 1. On Arm64 the p2m_ipa_bits could (theoretically) be 64-bit which, I assume, corresponds to the maximum guest physical address (1 << 64) - 1 = 0xFFFFFFFFFFFFFFFF. To store that GFN we need 52-bit. But, we provide 60-bit field which is more than enough, I think. Practically, the maximum supported p2m_ipa_bits is 48-bit, so the maximum supported GFN will occupy 36-bit only. Everything is ok here. 2. On Arm32 the p2m_ipa_bits is 40-bit which, I assume, corresponds to the maximum guest physical address (1 << 40) - 1 = 0xFFFFFFFFFF. To store that GFN we need 28-bit. If I did the calculation correctly, what we have on Arm32 is that PGT_INVALID_FRAME_GFN == maximum guest physical address and it looks like we need and extra bit on Arm32. Do you think we need to borrow one more bit from the count portion to stay on the safe side? > >> + * This only valid for the xenheap pages. >> + */ >> +#define PGT_gfn_width PG_shift(4) >> +#define PGT_gfn_mask ((1UL<<PGT_gfn_width)-1) > Any reason you don't use PG_mask() here? Any open-coding is prone > to people later making mistakes. I failed to come up with idea how to do that without #ifdef. As GFN starts at bit 0 different first parameter would be needed for PG_mask on 32-bit and 64-bit systems. I wonder whether PGC_count_mask/PGT_count_mask are open-coded on Arm/x86 because of the same reason.
On 16.09.2021 00:13, Oleksandr wrote: > On 15.09.21 13:06, Jan Beulich wrote: >> On 14.09.2021 22:44, Oleksandr Tyshchenko wrote: >>> --- a/xen/include/asm-arm/mm.h >>> +++ b/xen/include/asm-arm/mm.h >>> @@ -98,9 +98,18 @@ struct page_info >>> #define PGT_writable_page PG_mask(1, 1) /* has writable mappings? */ >>> #define PGT_type_mask PG_mask(1, 1) /* Bits 31 or 63. */ >>> >>> - /* Count of uses of this frame as its current type. */ >>> -#define PGT_count_width PG_shift(2) >>> -#define PGT_count_mask ((1UL<<PGT_count_width)-1) >>> + /* 3-bit count of uses of this frame as its current type. */ >>> +#define PGT_count_base PG_shift(4) >>> +#define PGT_count_mask PG_mask(7, 4) >>> + >>> +/* >>> + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table frame. >> I don't know enough Arm details to tell whether this is properly >> one bit more than the maximum number of physical address bits. >> Without the extra bit you wouldn't be able to tell apart a >> guest specified GFN matching the value of PGT_INVALID_FRAME_GFN >> from an entry which was set from INVALID_GFN. > Really good point. > > 1. On Arm64 the p2m_ipa_bits could (theoretically) be 64-bit which, I > assume, corresponds to the maximum guest physical address (1 << 64) - 1 > = 0xFFFFFFFFFFFFFFFF. > To store that GFN we need 52-bit. But, we provide 60-bit field which is > more than enough, I think. Practically, the maximum supported > p2m_ipa_bits is 48-bit, so the maximum supported GFN will occupy 36-bit > only. Everything is ok here. > 2. On Arm32 the p2m_ipa_bits is 40-bit which, I assume, corresponds to > the maximum guest physical address (1 << 40) - 1 = 0xFFFFFFFFFF. To > store that GFN we need 28-bit. If I did the calculation correctly, what > we have on Arm32 is that PGT_INVALID_FRAME_GFN == maximum guest physical > address and it looks like we need and extra bit on Arm32. Do you think > we need to borrow one more bit from the count portion to stay on the > safe side? I think so, unless there are further restrictions on the GFN range that I'm unaware of. For 64-bit, if you only need 52 bits, why do you make the field 60 bits wide? I'd recommend against "wasting" bits. Better keep the count field as wide as possible. >>> + * This only valid for the xenheap pages. >>> + */ >>> +#define PGT_gfn_width PG_shift(4) >>> +#define PGT_gfn_mask ((1UL<<PGT_gfn_width)-1) >> Any reason you don't use PG_mask() here? Any open-coding is prone >> to people later making mistakes. > I failed to come up with idea how to do that without #ifdef. As GFN > starts at bit 0 different first parameter would be needed for PG_mask on > 32-bit and 64-bit systems. > I wonder whether PGC_count_mask/PGT_count_mask are open-coded on Arm/x86 > because of the same reason. Hmm, that pre-existing open-coding isn't nice, but is perhaps a good enough reason to keep what you have. (Personally I wouldn't be afraid of adding an #ifdef here, but I realize views there may differ.) Jan
On 16.09.21 09:38, Jan Beulich wrote: Hi Jan > On 16.09.2021 00:13, Oleksandr wrote: >> On 15.09.21 13:06, Jan Beulich wrote: >>> On 14.09.2021 22:44, Oleksandr Tyshchenko wrote: >>>> --- a/xen/include/asm-arm/mm.h >>>> +++ b/xen/include/asm-arm/mm.h >>>> @@ -98,9 +98,18 @@ struct page_info >>>> #define PGT_writable_page PG_mask(1, 1) /* has writable mappings? */ >>>> #define PGT_type_mask PG_mask(1, 1) /* Bits 31 or 63. */ >>>> >>>> - /* Count of uses of this frame as its current type. */ >>>> -#define PGT_count_width PG_shift(2) >>>> -#define PGT_count_mask ((1UL<<PGT_count_width)-1) >>>> + /* 3-bit count of uses of this frame as its current type. */ >>>> +#define PGT_count_base PG_shift(4) >>>> +#define PGT_count_mask PG_mask(7, 4) >>>> + >>>> +/* >>>> + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table frame. >>> I don't know enough Arm details to tell whether this is properly >>> one bit more than the maximum number of physical address bits. >>> Without the extra bit you wouldn't be able to tell apart a >>> guest specified GFN matching the value of PGT_INVALID_FRAME_GFN >>> from an entry which was set from INVALID_GFN. >> Really good point. >> >> 1. On Arm64 the p2m_ipa_bits could (theoretically) be 64-bit which, I >> assume, corresponds to the maximum guest physical address (1 << 64) - 1 >> = 0xFFFFFFFFFFFFFFFF. >> To store that GFN we need 52-bit. But, we provide 60-bit field which is >> more than enough, I think. Practically, the maximum supported >> p2m_ipa_bits is 48-bit, so the maximum supported GFN will occupy 36-bit >> only. Everything is ok here. >> 2. On Arm32 the p2m_ipa_bits is 40-bit which, I assume, corresponds to >> the maximum guest physical address (1 << 40) - 1 = 0xFFFFFFFFFF. To >> store that GFN we need 28-bit. If I did the calculation correctly, what >> we have on Arm32 is that PGT_INVALID_FRAME_GFN == maximum guest physical >> address and it looks like we need and extra bit on Arm32. Do you think >> we need to borrow one more bit from the count portion to stay on the >> safe side? > I think so, unless there are further restrictions on the GFN range > that I'm unaware of. ok, thank you. > > For 64-bit, if you only need 52 bits, why do you make the field 60 > bits wide? I'd recommend against "wasting" bits. Better keep the > count field as wide as possible. Well, the reason almost the same as I already provided for why not using PG_mask for PGT_gfn_mask. Although we waste some bits on Arm64, having the same amount of bits for count on Arm32 and Arm64 let us avoid introducing an extra #ifdef to that header (if we go with maximum possible bits for count on each configuration we would need to have a set of PGT_count_*/PGT_gfn_*). I was thinking that if there was indeed a lack of bits for count then an extra #ifdef wouldn't be an argument at all. If I am not mistaken, the code which deals with count (where only 1 bit is used) is one and the same on both Arm32 and Arm64, so what is the point of diverging here (I mean to provide more bits for count on Arm64 because simply there is a reserve). But, if the reason I provided is weak I will update header to keep the count as wide as possible. > >>>> + * This only valid for the xenheap pages. >>>> + */ >>>> +#define PGT_gfn_width PG_shift(4) >>>> +#define PGT_gfn_mask ((1UL<<PGT_gfn_width)-1) >>> Any reason you don't use PG_mask() here? Any open-coding is prone >>> to people later making mistakes. >> I failed to come up with idea how to do that without #ifdef. As GFN >> starts at bit 0 different first parameter would be needed for PG_mask on >> 32-bit and 64-bit systems. >> I wonder whether PGC_count_mask/PGT_count_mask are open-coded on Arm/x86 >> because of the same reason. > Hmm, that pre-existing open-coding isn't nice, but is perhaps a > good enough reason to keep what you have. (Personally I wouldn't > be afraid of adding an #ifdef here, but I realize views there may > differ.) > > Jan >
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index eea926d..1857af4 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1376,14 +1376,18 @@ unsigned long domain_get_maximum_gpfn(struct domain *d) void share_xen_page_with_guest(struct page_info *page, struct domain *d, enum XENSHARE_flags flags) { + unsigned long type_info; + if ( page_get_owner(page) == d ) return; spin_lock(&d->page_alloc_lock); /* The incremented type count pins as writable or read-only. */ - page->u.inuse.type_info = - (flags == SHARE_ro ? PGT_none : PGT_writable_page) | 1; + type_info = page->u.inuse.type_info & ~(PGT_type_mask | PGT_count_mask); + page->u.inuse.type_info = type_info | + (flags == SHARE_ro ? PGT_none : PGT_writable_page) | + (1UL << PGT_count_base); page_set_owner(page, d); smp_wmb(); /* install valid domain ptr before updating refcnt. */ diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index eff9a10..a2b5597 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -718,8 +718,10 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m, gfn_t gfn, * TODO: Handle superpages, for now we only take special references for leaf * pages (specifically foreign ones, which can't be super mapped today). */ -static void p2m_put_l3_page(const lpae_t pte) +static void p2m_put_l3_page(struct p2m_domain *p2m, const lpae_t pte) { + mfn_t mfn = lpae_get_mfn(pte); + ASSERT(p2m_is_valid(pte)); /* @@ -731,11 +733,22 @@ static void p2m_put_l3_page(const lpae_t pte) */ if ( p2m_is_foreign(pte.p2m.type) ) { - mfn_t mfn = lpae_get_mfn(pte); - ASSERT(mfn_valid(mfn)); put_page(mfn_to_page(mfn)); } + +#ifdef CONFIG_GRANT_TABLE + /* + * Check whether we deal with grant table page. As the grant table page + * is xen_heap page and its entry has known p2m type, detect it and mark + * the stored GFN as invalid. Although this check is not precise and we + * might end up updating this for other xen_heap pages, this action is + * harmless to these pages since only grant table pages have this field + * in use. So, at worst, unnecessary action might be performed. + */ + if ( (pte.p2m.type == p2m_ram_rw) && is_xen_heap_mfn(mfn) ) + page_set_frame_gfn(mfn_to_page(mfn), INVALID_GFN); +#endif } /* Free lpae sub-tree behind an entry */ @@ -768,7 +781,7 @@ static void p2m_free_entry(struct p2m_domain *p2m, p2m->stats.mappings[level]--; /* Nothing to do if the entry is a super-page. */ if ( level == 3 ) - p2m_put_l3_page(entry); + p2m_put_l3_page(p2m, entry); return; } diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index e80f8d0..fd8d7e3 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -93,8 +93,6 @@ struct grant_table { /* Domain to which this struct grant_table belongs. */ const struct domain *domain; - - struct grant_table_arch arch; }; unsigned int __read_mostly opt_max_grant_frames = 64; @@ -1981,14 +1979,9 @@ int grant_table_init(struct domain *d, int max_grant_frames, grant_write_lock(gt); - ret = gnttab_init_arch(gt); - if ( ret ) - goto unlock; - /* gnttab_grow_table() allocates a min number of frames, so 0 is okay. */ ret = gnttab_grow_table(d, 0); - unlock: grant_write_unlock(gt); out: @@ -3894,8 +3887,6 @@ grant_table_destroy( if ( t == NULL ) return; - gnttab_destroy_arch(t); - for ( i = 0; i < nr_grant_frames(t); i++ ) free_xenheap_page(t->shared_raw[i]); xfree(t->shared_raw); diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 958ba0c..00371e7 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -2204,7 +2204,10 @@ void *alloc_xenheap_pages(unsigned int order, unsigned int memflags) return NULL; for ( i = 0; i < (1u << order); i++ ) + { pg[i].count_info |= PGC_xen_heap; + arch_alloc_xenheap_page(&pg[i]); + } return page_to_virt(pg); } @@ -2222,7 +2225,10 @@ void free_xenheap_pages(void *v, unsigned int order) pg = virt_to_page(v); for ( i = 0; i < (1u << order); i++ ) + { pg[i].count_info &= ~PGC_xen_heap; + arch_free_xenheap_page(&pg[i]); + } free_heap_pages(pg, order, true); } diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h index 0ce77f9..479339d 100644 --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -11,11 +11,6 @@ #define INITIAL_NR_GRANT_FRAMES 1U #define GNTTAB_MAX_VERSION 1 -struct grant_table_arch { - gfn_t *shared_gfn; - gfn_t *status_gfn; -}; - static inline void gnttab_clear_flags(struct domain *d, unsigned int mask, uint16_t *addr) { @@ -46,35 +41,11 @@ int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn, #define gnttab_dom0_frames() \ min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext)) -#define gnttab_init_arch(gt) \ -({ \ - unsigned int ngf_ = (gt)->max_grant_frames; \ - unsigned int nsf_ = grant_to_status_frames(ngf_); \ - \ - (gt)->arch.shared_gfn = xmalloc_array(gfn_t, ngf_); \ - (gt)->arch.status_gfn = xmalloc_array(gfn_t, nsf_); \ - if ( (gt)->arch.shared_gfn && (gt)->arch.status_gfn ) \ - { \ - while ( ngf_-- ) \ - (gt)->arch.shared_gfn[ngf_] = INVALID_GFN; \ - while ( nsf_-- ) \ - (gt)->arch.status_gfn[nsf_] = INVALID_GFN; \ - } \ - else \ - gnttab_destroy_arch(gt); \ - (gt)->arch.shared_gfn ? 0 : -ENOMEM; \ -}) - -#define gnttab_destroy_arch(gt) \ - do { \ - XFREE((gt)->arch.shared_gfn); \ - XFREE((gt)->arch.status_gfn); \ - } while ( 0 ) - #define gnttab_set_frame_gfn(gt, st, idx, gfn) \ do { \ - ((st) ? (gt)->arch.status_gfn : (gt)->arch.shared_gfn)[idx] = \ - (gfn); \ + struct page_info *pg_ = (st) ? gnttab_status_page(gt, idx) \ + : gnttab_shared_page(gt, idx); \ + page_set_frame_gfn(pg_, gfn); \ } while ( 0 ) #define gnttab_get_frame_gfn(gt, st, idx) ({ \ @@ -82,11 +53,25 @@ int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn, : gnttab_shared_gfn(NULL, gt, idx); \ }) -#define gnttab_shared_gfn(d, t, i) \ - (((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i]) +#define gnttab_shared_page(t, i) ({ \ + ASSERT((t)->shared_raw[i]); \ + mfn_to_page(_mfn(__virt_to_mfn((t)->shared_raw[i]))); \ +}) + +#define gnttab_status_page(t, i) ({ \ + ASSERT((t)->status[i]); \ + mfn_to_page(_mfn(__virt_to_mfn((t)->status[i]))); \ +}) -#define gnttab_status_gfn(d, t, i) \ - (((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i]) +#define gnttab_shared_gfn(d, t, i) ({ \ + struct page_info *pg_ = gnttab_shared_page(t, i); \ + page_get_frame_gfn(pg_); \ +}) + +#define gnttab_status_gfn(d, t, i) ({ \ + struct page_info *pg_ = gnttab_status_page(t, i); \ + page_get_frame_gfn(pg_); \ +}) #define gnttab_need_iommu_mapping(d) \ (is_domain_direct_mapped(d) && is_iommu_enabled(d)) diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index ded74d2..dd425d6 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -98,9 +98,18 @@ struct page_info #define PGT_writable_page PG_mask(1, 1) /* has writable mappings? */ #define PGT_type_mask PG_mask(1, 1) /* Bits 31 or 63. */ - /* Count of uses of this frame as its current type. */ -#define PGT_count_width PG_shift(2) -#define PGT_count_mask ((1UL<<PGT_count_width)-1) + /* 3-bit count of uses of this frame as its current type. */ +#define PGT_count_base PG_shift(4) +#define PGT_count_mask PG_mask(7, 4) + +/* + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table frame. + * This only valid for the xenheap pages. + */ +#define PGT_gfn_width PG_shift(4) +#define PGT_gfn_mask ((1UL<<PGT_gfn_width)-1) + +#define PGT_INVALID_FRAME_GFN _gfn(PGT_gfn_mask) /* Cleared when the owning guest 'frees' this page. */ #define _PGC_allocated PG_shift(1) @@ -163,6 +172,27 @@ extern unsigned long xenheap_base_pdx; #define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma)))) +#define page_get_frame_gfn(p) ({ \ + gfn_t gfn_ = _gfn((p)->u.inuse.type_info & PGT_gfn_mask); \ + gfn_eq(gfn_, PGT_INVALID_FRAME_GFN) ? INVALID_GFN : gfn_; \ +}) + +#define page_set_frame_gfn(p, gfn) ({ \ + gfn_t gfn_ = gfn_eq(gfn, INVALID_GFN) ? \ + PGT_INVALID_FRAME_GFN : gfn; \ + (p)->u.inuse.type_info &= ~PGT_gfn_mask; \ + (p)->u.inuse.type_info |= gfn_x(gfn_); \ +}) + +/* + * As the struct page_info representing the xen_heap page can contain + * the grant table frame GFN on Arm we need to clear it beforehand and + * make sure it is not still set when freeing a page. + */ +#define arch_alloc_xenheap_page(p) page_set_frame_gfn(p, INVALID_GFN) +#define arch_free_xenheap_page(p) \ + BUG_ON(!gfn_eq(page_get_frame_gfn(p), INVALID_GFN)) + #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START) /* PDX of the first page in the frame table. */ extern unsigned long frametable_base_pdx; diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h index 84e3296..0eb018f 100644 --- a/xen/include/asm-x86/grant_table.h +++ b/xen/include/asm-x86/grant_table.h @@ -14,9 +14,6 @@ #define INITIAL_NR_GRANT_FRAMES 1U -struct grant_table_arch { -}; - static inline int create_grant_host_mapping(uint64_t addr, mfn_t frame, unsigned int flags, unsigned int cache_flags) @@ -35,8 +32,6 @@ static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame, return replace_grant_pv_mapping(addr, frame, new_addr, flags); } -#define gnttab_init_arch(gt) 0 -#define gnttab_destroy_arch(gt) do {} while ( 0 ) #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 ) #define gnttab_get_frame_gfn(gt, st, idx) ({ \ mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h index cb90527..04d8704 100644 --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -327,6 +327,10 @@ struct page_info #define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma)))) +/* No arch-specific actions are needed for the xen_heap page */ +#define arch_alloc_xenheap_page(p) do {} while ( 0 ) +#define arch_free_xenheap_page(p) do {} while ( 0 ) + #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START) extern unsigned long max_page; extern unsigned long total_pages;