[3/3] x86/PV: polish pv_set_gdt()

Commit Message

Jan Beulich Dec. 6, 2019, 10:15 a.m. UTC

There's no need to invoke get_page_from_gfn(), and there's also no need
to update the passed in frames[]. Invoke get_page_and_type() directly.

Also make the function's frames[] parameter const, change its return
type to int, and drop the bogus casts from two of its invocations.

Finally a little bit of cosmetics.

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Andrew Cooper Dec. 6, 2019, 10:36 a.m. UTC | #1

On 06/12/2019 10:15, Jan Beulich wrote:
> There's no need to invoke get_page_from_gfn(), and there's also no need
> to update the passed in frames[]. Invoke get_page_and_type() directly.
>
> Also make the function's frames[] parameter const, change its return
> type to int, and drop the bogus casts from two of its invocations.
>
> Finally a little bit of cosmetics.
>
> Signed-off-by: Jan Beulich <[e-mail]>

Acked-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1089,7 +1089,7 @@  int arch_set_info_guest(
         return rc;
 
     if ( !compat )
-        rc = (int)pv_set_gdt(v, c.nat->gdt_frames, c.nat->gdt_ents);
+        rc = pv_set_gdt(v, c.nat->gdt_frames, c.nat->gdt_ents);
     else
     {
         unsigned long gdt_frames[ARRAY_SIZE(v->arch.pv.gdt_frames)];
@@ -1100,7 +1100,7 @@  int arch_set_info_guest(
         for ( i = 0; i < nr_gdt_frames; ++i )
             gdt_frames[i] = c.cmp->gdt_frames[i];
 
-        rc = (int)pv_set_gdt(v, gdt_frames, c.cmp->gdt_ents);
+        rc = pv_set_gdt(v, gdt_frames, c.cmp->gdt_ents);
     }
     if ( rc != 0 )
         return rc;
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -96,7 +96,8 @@  void pv_destroy_gdt(struct vcpu *v)
     }
 }
 
-long pv_set_gdt(struct vcpu *v, unsigned long *frames, unsigned int entries)
+int pv_set_gdt(struct vcpu *v, const unsigned long frames[],
+               unsigned int entries)
 {
     struct domain *d = v->domain;
     l1_pgentry_t *pl1e;
@@ -110,17 +111,11 @@  long pv_set_gdt(struct vcpu *v, unsigned
     /* Check the pages in the new GDT. */
     for ( i = 0; i < nr_frames; i++ )
     {
-        struct page_info *page;
+        mfn_t mfn = _mfn(frames[i]);
 
-        page = get_page_from_gfn(d, frames[i], NULL, P2M_ALLOC);
-        if ( !page )
+        if ( !mfn_valid(mfn) ||
+             !get_page_and_type(mfn_to_page(mfn), d, PGT_seg_desc_page) )
             goto fail;
-        if ( !get_page_type(page, PGT_seg_desc_page) )
-        {
-            put_page(page);
-            goto fail;
-        }
-        frames[i] = mfn_x(page_to_mfn(page));
     }
 
     /* Tear down the old GDT. */
@@ -139,9 +134,8 @@  long pv_set_gdt(struct vcpu *v, unsigned
 
  fail:
     while ( i-- > 0 )
-    {
         put_page_and_type(mfn_to_page(_mfn(frames[i])));
-    }
+
     return -EINVAL;
 }
 
--- a/xen/include/asm-x86/pv/mm.h
+++ b/xen/include/asm-x86/pv/mm.h
@@ -25,7 +25,8 @@ 
 
 int pv_ro_page_fault(unsigned long addr, struct cpu_user_regs *regs);
 
-long pv_set_gdt(struct vcpu *v, unsigned long *frames, unsigned int entries);
+int pv_set_gdt(struct vcpu *v, const unsigned long frames[],
+               unsigned int entries);
 void pv_destroy_gdt(struct vcpu *v);
 
 bool pv_map_ldt_shadow_page(unsigned int off);
@@ -43,8 +44,8 @@  static inline int pv_ro_page_fault(unsig
     return 0;
 }
 
-static inline long pv_set_gdt(struct vcpu *v, unsigned long *frames,
-                              unsigned int entries)
+static inline int pv_set_gdt(struct vcpu *v, const unsigned long frames[],
+                             unsigned int entries)
 { ASSERT_UNREACHABLE(); return -EINVAL; }
 static inline void pv_destroy_gdt(struct vcpu *v) { ASSERT_UNREACHABLE(); }