Re: [Xen-devel] [PATCH 20/34] x86/mtrr: move is_var_mtrr_overlapped

2018-08-21 Thread Wei Liu 
On Tue, Aug 21, 2018 at 01:58:51AM -0600, Jan Beulich wrote:
> >>> On 17.08.18 at 17:12,  wrote:
> > Move it to x86 generic code. While at it, use proper boolean type.
> > 
> > Signed-off-by: Wei Liu 
> 
> Acked-by: Jan Beulich 
> with a couple of cosmetic adjustments beyond the bool
> conversion you've already done:
> 
> > +bool is_var_mtrr_overlapped(const struct mtrr_state *m)
> > +{
> > +unsigned int seg, i;
> > +unsigned int num_var_ranges = MASK_EXTR(m->mtrr_cap, MTRRcap_VCNT);
> > +
> > +for ( i = 0; i < num_var_ranges; i++ )
> > +{
> > +uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
> > +uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;
> 
> Here and below I wonder whether PFN_DOWN() wouldn't be
> appropriate to use. I'll leave that up to you though.
> 

Since base and mask aren't really PFN so I think it would be more
appropriate to leave them as-is.

I have fixed up the other two issues you mentioned.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 20/34] x86/mtrr: move is_var_mtrr_overlapped

2018-08-21 Thread Jan Beulich 
>>> On 17.08.18 at 17:12,  wrote:
> Move it to x86 generic code. While at it, use proper boolean type.
> 
> Signed-off-by: Wei Liu 

Acked-by: Jan Beulich 
with a couple of cosmetic adjustments beyond the bool
conversion you've already done:

> +bool is_var_mtrr_overlapped(const struct mtrr_state *m)
> +{
> +unsigned int seg, i;
> +unsigned int num_var_ranges = MASK_EXTR(m->mtrr_cap, MTRRcap_VCNT);
> +
> +for ( i = 0; i < num_var_ranges; i++ )
> +{
> +uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
> +uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;

Here and below I wonder whether PFN_DOWN() wouldn't be
appropriate to use. I'll leave that up to you though.

> +if ( !(m->var_ranges[i].mask & MTRR_PHYSMASK_VALID) )
> +continue;
> +
> +for ( seg = i + 1; seg < num_var_ranges; seg ++ )

Stray blank before ++.

> +{
> +uint64_t base2 = m->var_ranges[seg].base >> PAGE_SHIFT;
> +uint64_t mask2 = m->var_ranges[seg].mask >> PAGE_SHIFT;
> +
> +if ( !(m->var_ranges[seg].mask & MTRR_PHYSMASK_VALID) )
> +continue;
> +
> +if ( (base1 & mask1 & mask2) == (base2 & mask2 & mask1) )
> +{
> +/* MTRR is overlapped. */

"MTRRs overlap" or some such.

> +return true;
> +}
> +}
> +}
> +return false;
> +}

Blank line ahead of main "return" please.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 20/34] x86/mtrr: move is_var_mtrr_overlapped

2018-08-17 Thread Wei Liu 
Move it to x86 generic code. While at it, use proper boolean type.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/cpu/mtrr/generic.c | 31 +++
 xen/arch/x86/hvm/mtrr.c | 31 ---
 xen/include/asm-x86/mtrr.h  |  2 +-
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index 0976365..499d5f4 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -51,6 +51,37 @@ get_fixed_ranges(mtrr_type * frs)
}
 }
 
+bool is_var_mtrr_overlapped(const struct mtrr_state *m)
+{
+unsigned int seg, i;
+unsigned int num_var_ranges = MASK_EXTR(m->mtrr_cap, MTRRcap_VCNT);
+
+for ( i = 0; i < num_var_ranges; i++ )
+{
+uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
+uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;
+
+if ( !(m->var_ranges[i].mask & MTRR_PHYSMASK_VALID) )
+continue;
+
+for ( seg = i + 1; seg < num_var_ranges; seg ++ )
+{
+uint64_t base2 = m->var_ranges[seg].base >> PAGE_SHIFT;
+uint64_t mask2 = m->var_ranges[seg].mask >> PAGE_SHIFT;
+
+if ( !(m->var_ranges[seg].mask & MTRR_PHYSMASK_VALID) )
+continue;
+
+if ( (base1 & mask1 & mask2) == (base2 & mask2 & mask1) )
+{
+/* MTRR is overlapped. */
+return true;
+}
+}
+}
+return false;
+}
+
 void mtrr_save_fixed_ranges(void *info)
 {
get_fixed_ranges(mtrr_state.fixed_ranges);
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index c502dda..edfe5cd 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -75,37 +75,6 @@ static uint8_t __read_mostly 
mtrr_epat_tbl[MTRR_NUM_TYPES][MEMORY_NUM_TYPES] =
 static uint8_t __read_mostly pat_entry_tbl[PAT_TYPE_NUMS] =
 { [0 ... PAT_TYPE_NUMS-1] = INVALID_MEM_TYPE };
 
-bool_t is_var_mtrr_overlapped(const struct mtrr_state *m)
-{
-unsigned int seg, i;
-unsigned int num_var_ranges = MASK_EXTR(m->mtrr_cap, MTRRcap_VCNT);
-
-for ( i = 0; i < num_var_ranges; i++ )
-{
-uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
-uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;
-
-if ( !(m->var_ranges[i].mask & MTRR_PHYSMASK_VALID) )
-continue;
-
-for ( seg = i + 1; seg < num_var_ranges; seg ++ )
-{
-uint64_t base2 = m->var_ranges[seg].base >> PAGE_SHIFT;
-uint64_t mask2 = m->var_ranges[seg].mask >> PAGE_SHIFT;
-
-if ( !(m->var_ranges[seg].mask & MTRR_PHYSMASK_VALID) )
-continue;
-
-if ( (base1 & mask1 & mask2) == (base2 & mask2 & mask1) )
-{
-/* MTRR is overlapped. */
-return 1;
-}
-}
-}
-return 0;
-}
-
 static int __init hvm_mtrr_pat_init(void)
 {
 unsigned int i, j;
diff --git a/xen/include/asm-x86/mtrr.h b/xen/include/asm-x86/mtrr.h
index 72d0690..7edcb94 100644
--- a/xen/include/asm-x86/mtrr.h
+++ b/xen/include/asm-x86/mtrr.h
@@ -95,7 +95,7 @@ extern bool_t mtrr_def_type_msr_set(struct domain *, struct 
mtrr_state *,
 extern void memory_type_changed(struct domain *);
 extern bool_t pat_msr_set(uint64_t *pat, uint64_t msr);
 
-bool_t is_var_mtrr_overlapped(const struct mtrr_state *m);
+bool is_var_mtrr_overlapped(const struct mtrr_state *m);
 bool mtrr_pat_not_equal(const struct vcpu *vd, const struct vcpu *vs);
 
 #endif /* __ASM_X86_MTRR_H__ */
-- 
git-series 0.9.1

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel