Having grant table code in arch/x86/mm.c seems wrong. Move it to the
new file arch/x86/gnttab.c, especially as the amount of code is
expected to grow further.

No functional change.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/gnttab.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm.c     | 47 ----------------------------
 2 files changed, 78 insertions(+), 47 deletions(-)
 create mode 100644 arch/x86/gnttab.c

diff --git a/arch/x86/gnttab.c b/arch/x86/gnttab.c
new file mode 100644
index 0000000..56e59d7
--- /dev/null
+++ b/arch/x86/gnttab.c
@@ -0,0 +1,78 @@
+/* -*-  Mode:C; c-basic-offset:4; tab-width:4 -*-
+ *
+ * (C) 2021 - Juergen Gross, SUSE Software Solutions Germany GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <mini-os/os.h>
+#include <mini-os/hypervisor.h>
+#include <mini-os/gnttab.h>
+#include <mini-os/mm.h>
+#include <mini-os/types.h>
+
+grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
+{
+    struct gnttab_setup_table setup;
+    unsigned long frames[nr_grant_frames];
+
+    setup.dom = DOMID_SELF;
+    setup.nr_frames = nr_grant_frames;
+    set_xen_guest_handle(setup.frame_list, frames);
+
+    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+    return map_frames(frames, nr_grant_frames);
+}
+
+void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
+{
+#ifdef CONFIG_PARAVIRT
+    int i;
+
+    for ( i = 0; i < nr_grant_frames; i++ )
+    {
+        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
+                __pte(0x0 << PAGE_SHIFT), UVMF_INVLPG);
+    }
+#endif
+    return;
+}
+
+void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
+{
+    struct gnttab_setup_table setup;
+    unsigned long frames[nr_grant_frames];
+#ifdef CONFIG_PARAVIRT
+    int i;
+#endif
+
+    setup.dom = DOMID_SELF;
+    setup.nr_frames = nr_grant_frames;
+    set_xen_guest_handle(setup.frame_list, frames);
+
+    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+
+#ifdef CONFIG_PARAVIRT
+    for ( i = 0; i < nr_grant_frames; i++ )
+    {
+        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
+                __pte((frames[i] << PAGE_SHIFT) | L1_PROT), UVMF_INVLPG);
+    }
+#endif
+}
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index c30d8bc..220c0b4 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -837,53 +837,6 @@ void arch_init_mm(unsigned long* start_pfn_p, unsigned long* max_pfn_p)
 #endif
 }
 
-grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
-{
-    struct gnttab_setup_table setup;
-    unsigned long frames[nr_grant_frames];
-
-    setup.dom = DOMID_SELF;
-    setup.nr_frames = nr_grant_frames;
-    set_xen_guest_handle(setup.frame_list, frames);
-
-    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
-    return map_frames(frames, nr_grant_frames);
-}
-
-void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
-{
-#ifdef CONFIG_PARAVIRT
-    int i;
-
-    for (i = 0; i < nr_grant_frames; i++) {
-        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
-                (pte_t){0x0<<PAGE_SHIFT}, UVMF_INVLPG);
-    }
-#endif
-    return;
-}
-
-void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
-{
-    struct gnttab_setup_table setup;
-    unsigned long frames[nr_grant_frames];
-#ifdef CONFIG_PARAVIRT
-    int i;
-#endif
-    setup.dom = DOMID_SELF;
-    setup.nr_frames = nr_grant_frames;
-    set_xen_guest_handle(setup.frame_list, frames);
-
-    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
-
-#ifdef CONFIG_PARAVIRT
-    for (i = 0; i < nr_grant_frames; i++) {
-        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
-                (pte_t){(frames[i] << PAGE_SHIFT) | L1_PROT}, UVMF_INVLPG);
-    }
-#endif
-}
-
 unsigned long alloc_virt_kernel(unsigned n_pages)
 {
     unsigned long addr;
-- 
2.26.2

Juergen Gross, le lun. 06 déc. 2021 08:23:34 +0100, a ecrit:
> Having grant table code in arch/x86/mm.c seems wrong. Move it to the
> new file arch/x86/gnttab.c, especially as the amount of code is
> expected to grow further.
> 
> No functional change.

There is the __pte fix that you'd probably want to mention.

> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  arch/x86/gnttab.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/mm.c     | 47 ----------------------------
>  2 files changed, 78 insertions(+), 47 deletions(-)
>  create mode 100644 arch/x86/gnttab.c
> 
> diff --git a/arch/x86/gnttab.c b/arch/x86/gnttab.c
> new file mode 100644
> index 0000000..56e59d7
> --- /dev/null
> +++ b/arch/x86/gnttab.c
> @@ -0,0 +1,78 @@
> +/* -*-  Mode:C; c-basic-offset:4; tab-width:4 -*-
> + *
> + * (C) 2021 - Juergen Gross, SUSE Software Solutions Germany GmbH
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <mini-os/os.h>
> +#include <mini-os/hypervisor.h>
> +#include <mini-os/gnttab.h>
> +#include <mini-os/mm.h>
> +#include <mini-os/types.h>
> +
> +grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
> +{
> +    struct gnttab_setup_table setup;
> +    unsigned long frames[nr_grant_frames];
> +
> +    setup.dom = DOMID_SELF;
> +    setup.nr_frames = nr_grant_frames;
> +    set_xen_guest_handle(setup.frame_list, frames);
> +
> +    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
> +    return map_frames(frames, nr_grant_frames);
> +}
> +
> +void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
> +{
> +#ifdef CONFIG_PARAVIRT
> +    int i;
> +
> +    for ( i = 0; i < nr_grant_frames; i++ )
> +    {
> +        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
> +                __pte(0x0 << PAGE_SHIFT), UVMF_INVLPG);
> +    }
> +#endif
> +    return;
> +}
> +
> +void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
> +{
> +    struct gnttab_setup_table setup;
> +    unsigned long frames[nr_grant_frames];
> +#ifdef CONFIG_PARAVIRT
> +    int i;
> +#endif
> +
> +    setup.dom = DOMID_SELF;
> +    setup.nr_frames = nr_grant_frames;
> +    set_xen_guest_handle(setup.frame_list, frames);
> +
> +    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
> +
> +#ifdef CONFIG_PARAVIRT
> +    for ( i = 0; i < nr_grant_frames; i++ )
> +    {
> +        HYPERVISOR_update_va_mapping((unsigned long)gnttab_table + PAGE_SIZE * i,
> +                __pte((frames[i] << PAGE_SHIFT) | L1_PROT), UVMF_INVLPG);
> +    }
> +#endif
> +}
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index c30d8bc..220c0b4 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -837,53 +837,6 @@ void arch_init_mm(unsigned long* start_pfn_p, unsigned long* max_pfn_p)
>  #endif
>  }
>  
> -grant_entry_v1_t *arch_init_gnttab(int nr_grant_frames)
> -{
> -    struct gnttab_setup_table setup;
> -    unsigned long frames[nr_grant_frames];
> -
> -    setup.dom = DOMID_SELF;
> -    setup.nr_frames = nr_grant_frames;
> -    set_xen_guest_handle(setup.frame_list, frames);
> -
> -    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
> -    return map_frames(frames, nr_grant_frames);
> -}
> -
> -void arch_suspend_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
> -{
> -#ifdef CONFIG_PARAVIRT
> -    int i;
> -
> -    for (i = 0; i < nr_grant_frames; i++) {
> -        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
> -                (pte_t){0x0<<PAGE_SHIFT}, UVMF_INVLPG);
> -    }
> -#endif
> -    return;
> -}
> -
> -void arch_resume_gnttab(grant_entry_v1_t *gnttab_table, int nr_grant_frames)
> -{
> -    struct gnttab_setup_table setup;
> -    unsigned long frames[nr_grant_frames];
> -#ifdef CONFIG_PARAVIRT
> -    int i;
> -#endif
> -    setup.dom = DOMID_SELF;
> -    setup.nr_frames = nr_grant_frames;
> -    set_xen_guest_handle(setup.frame_list, frames);
> -
> -    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
> -
> -#ifdef CONFIG_PARAVIRT
> -    for (i = 0; i < nr_grant_frames; i++) {
> -        HYPERVISOR_update_va_mapping((unsigned long)(((char *)gnttab_table) + PAGE_SIZE * i),
> -                (pte_t){(frames[i] << PAGE_SHIFT) | L1_PROT}, UVMF_INVLPG);
> -    }
> -#endif
> -}
> -
>  unsigned long alloc_virt_kernel(unsigned n_pages)
>  {
>      unsigned long addr;
> -- 
> 2.26.2
> 

-- 
Samuel
War doesn't prove who's right, just who's left.