|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/6] x86/shadow: widen reference count
On 12/12/2017 03:07 PM, Jan Beulich wrote:
> Utilize as many of the bits available in the union as possible, without
> (just to be on the safe side) colliding with any of the bits outside of
> PGT_type_mask.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/mm/shadow/private.h
> +++ b/xen/arch/x86/mm/shadow/private.h
> @@ -510,7 +510,7 @@ void sh_destroy_shadow(struct domain *d,
> * Returns 0 for failure, 1 for success. */
> static inline int sh_get_ref(struct domain *d, mfn_t smfn, paddr_t entry_pa)
> {
> - u32 x, nx;
> + unsigned long x, nx;
> struct page_info *sp = mfn_to_page(smfn);
>
> ASSERT(mfn_valid(smfn));
> @@ -519,7 +519,7 @@ static inline int sh_get_ref(struct doma
> x = sp->u.sh.count;
> nx = x + 1;
>
> - if ( unlikely(nx >= (1U << PAGE_SH_REFCOUNT_WIDTH)) )
> + if ( unlikely(nx >= (1UL << PAGE_SH_REFCOUNT_WIDTH)) )
> {
> SHADOW_PRINTK("shadow ref overflow, gmfn=%lx smfn=%lx\n",
> __backpointer(sp), mfn_x(smfn));
> @@ -543,7 +543,7 @@ static inline int sh_get_ref(struct doma
> * physical address of the shadow entry that held this reference. */
> static inline void sh_put_ref(struct domain *d, mfn_t smfn, paddr_t entry_pa)
> {
> - u32 x, nx;
> + unsigned long x, nx;
> struct page_info *sp = mfn_to_page(smfn);
>
> ASSERT(mfn_valid(smfn));
> @@ -561,8 +561,8 @@ static inline void sh_put_ref(struct dom
>
> if ( unlikely(x == 0) )
> {
> - SHADOW_ERROR("shadow ref underflow, smfn=%lx oc=%08x t=%#x\n",
> - mfn_x(smfn), sp->u.sh.count, sp->u.sh.type);
> + SHADOW_ERROR("shadow ref underflow, smfn=%lx oc=%#lx t=%#x\n",
> + mfn_x(smfn), sp->u.sh.count + 0UL, sp->u.sh.type);
> BUG();
> }
>
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -18,6 +18,77 @@
> */
> #define PFN_ORDER(_pfn) ((_pfn)->v.free.order)
>
> +#define PG_shift(idx) (BITS_PER_LONG - (idx))
> +#define PG_mask(x, idx) (x ## UL << PG_shift(idx))
> +
> + /* The following page types are MUTUALLY EXCLUSIVE. */
> +#define PGT_none PG_mask(0, 3) /* no special uses of this page */
> +#define PGT_l1_page_table PG_mask(1, 3) /* using as an L1 page table? */
> +#define PGT_l2_page_table PG_mask(2, 3) /* using as an L2 page table? */
> +#define PGT_l3_page_table PG_mask(3, 3) /* using as an L3 page table? */
> +#define PGT_l4_page_table PG_mask(4, 3) /* using as an L4 page table? */
> +#define PGT_seg_desc_page PG_mask(5, 3) /* using this page in a GDT/LDT? */
> +#define PGT_shared_page PG_mask(6, 3) /* CoW sharable page */
> +#define PGT_writable_page PG_mask(7, 3) /* has writable mappings? */
> +#define PGT_type_mask PG_mask(7, 3) /* Bits 61-63. */
> +
> + /* Page is locked? */
> +#define _PGT_locked PG_shift(4)
> +#define PGT_locked PG_mask(1, 4)
> + /* Owning guest has pinned this page to its current type? */
> +#define _PGT_pinned PG_shift(5)
> +#define PGT_pinned PG_mask(1, 5)
> + /* Has this page been validated for use as its current type? */
> +#define _PGT_validated PG_shift(6)
> +#define PGT_validated PG_mask(1, 6)
> + /* PAE only: is this an L2 page directory containing Xen-private mappings?
> */
> +#define _PGT_pae_xen_l2 PG_shift(7)
> +#define PGT_pae_xen_l2 PG_mask(1, 7)
> +/* Has this page been *partially* validated for use as its current type? */
> +#define _PGT_partial PG_shift(8)
> +#define PGT_partial PG_mask(1, 8)
> +
> + /* Count of uses of this frame as its current type. */
> +#define PGT_count_width PG_shift(8)
> +#define PGT_count_mask ((1UL<<PGT_count_width)-1)
> +
> +/* Are the 'type mask' bits identical? */
> +#define PGT_type_equal(x, y) (!(((x) ^ (y)) & PGT_type_mask))
> +
> + /* Cleared when the owning guest 'frees' this page. */
> +#define _PGC_allocated PG_shift(1)
> +#define PGC_allocated PG_mask(1, 1)
> + /* Page is Xen heap? */
> +#define _PGC_xen_heap PG_shift(2)
> +#define PGC_xen_heap PG_mask(1, 2)
> + /* Set when is using a page as a page table */
> +#define _PGC_page_table PG_shift(3)
> +#define PGC_page_table PG_mask(1, 3)
> + /* 3-bit PAT/PCD/PWT cache-attribute hint. */
> +#define PGC_cacheattr_base PG_shift(6)
> +#define PGC_cacheattr_mask PG_mask(7, 6)
> + /* Page is broken? */
> +#define _PGC_broken PG_shift(7)
> +#define PGC_broken PG_mask(1, 7)
> + /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */
> +#define PGC_state PG_mask(3, 9)
> +#define PGC_state_inuse PG_mask(0, 9)
> +#define PGC_state_offlining PG_mask(1, 9)
> +#define PGC_state_offlined PG_mask(2, 9)
> +#define PGC_state_free PG_mask(3, 9)
> +#define page_state_is(pg, st) (((pg)->count_info&PGC_state) ==
> PGC_state_##st)
> +
> + /* Count of references to this frame. */
> +#define PGC_count_width PG_shift(9)
> +#define PGC_count_mask ((1UL<<PGC_count_width)-1)
> +
> +/*
> + * Page needs to be scrubbed. Since this bit can only be set on a page that
> is
> + * free (i.e. in PGC_state_free) we can reuse PGC_allocated bit.
> + */
> +#define _PGC_need_scrub _PGC_allocated
> +#define PGC_need_scrub PGC_allocated
> +
> #ifndef CONFIG_BIGMEM
> /*
> * This definition is solely for the use in struct page_info (and
> @@ -82,7 +153,7 @@ struct page_info
> unsigned long type:5; /* What kind of shadow is this? */
> unsigned long pinned:1; /* Is the shadow pinned? */
> unsigned long head:1; /* Is this the first page of the shadow?
> */
> -#define PAGE_SH_REFCOUNT_WIDTH 25
> +#define PAGE_SH_REFCOUNT_WIDTH (PGT_count_width - 7)
> unsigned long count:PAGE_SH_REFCOUNT_WIDTH; /* Reference count */
> } sh;
>
> @@ -198,77 +269,6 @@ struct page_info
>
> #undef __pdx_t
>
> -#define PG_shift(idx) (BITS_PER_LONG - (idx))
> -#define PG_mask(x, idx) (x ## UL << PG_shift(idx))
> -
> - /* The following page types are MUTUALLY EXCLUSIVE. */
> -#define PGT_none PG_mask(0, 3) /* no special uses of this page */
> -#define PGT_l1_page_table PG_mask(1, 3) /* using as an L1 page table? */
> -#define PGT_l2_page_table PG_mask(2, 3) /* using as an L2 page table? */
> -#define PGT_l3_page_table PG_mask(3, 3) /* using as an L3 page table? */
> -#define PGT_l4_page_table PG_mask(4, 3) /* using as an L4 page table? */
> -#define PGT_seg_desc_page PG_mask(5, 3) /* using this page in a GDT/LDT? */
> -#define PGT_shared_page PG_mask(6, 3) /* CoW sharable page */
> -#define PGT_writable_page PG_mask(7, 3) /* has writable mappings? */
> -#define PGT_type_mask PG_mask(7, 3) /* Bits 61-63. */
> -
> - /* Page is locked? */
> -#define _PGT_locked PG_shift(4)
> -#define PGT_locked PG_mask(1, 4)
> - /* Owning guest has pinned this page to its current type? */
> -#define _PGT_pinned PG_shift(5)
> -#define PGT_pinned PG_mask(1, 5)
> - /* Has this page been validated for use as its current type? */
> -#define _PGT_validated PG_shift(6)
> -#define PGT_validated PG_mask(1, 6)
> - /* PAE only: is this an L2 page directory containing Xen-private mappings?
> */
> -#define _PGT_pae_xen_l2 PG_shift(7)
> -#define PGT_pae_xen_l2 PG_mask(1, 7)
> -/* Has this page been *partially* validated for use as its current type? */
> -#define _PGT_partial PG_shift(8)
> -#define PGT_partial PG_mask(1, 8)
> -
> - /* Count of uses of this frame as its current type. */
> -#define PGT_count_width PG_shift(8)
> -#define PGT_count_mask ((1UL<<PGT_count_width)-1)
> -
> -/* Are the 'type mask' bits identical? */
> -#define PGT_type_equal(x, y) (!(((x) ^ (y)) & PGT_type_mask))
> -
> - /* Cleared when the owning guest 'frees' this page. */
> -#define _PGC_allocated PG_shift(1)
> -#define PGC_allocated PG_mask(1, 1)
> - /* Page is Xen heap? */
> -#define _PGC_xen_heap PG_shift(2)
> -#define PGC_xen_heap PG_mask(1, 2)
> - /* Set when is using a page as a page table */
> -#define _PGC_page_table PG_shift(3)
> -#define PGC_page_table PG_mask(1, 3)
> - /* 3-bit PAT/PCD/PWT cache-attribute hint. */
> -#define PGC_cacheattr_base PG_shift(6)
> -#define PGC_cacheattr_mask PG_mask(7, 6)
> - /* Page is broken? */
> -#define _PGC_broken PG_shift(7)
> -#define PGC_broken PG_mask(1, 7)
> - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */
> -#define PGC_state PG_mask(3, 9)
> -#define PGC_state_inuse PG_mask(0, 9)
> -#define PGC_state_offlining PG_mask(1, 9)
> -#define PGC_state_offlined PG_mask(2, 9)
> -#define PGC_state_free PG_mask(3, 9)
> -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) ==
> PGC_state_##st)
What's the point of moving this code? And are there any important changes?
It would be a lot easier to review if you separated code motion from
code changes; but if you don't want to do that, you need to make it
clear what you're doing in your changelog.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |