[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67503f2a-6d93-59c6-5a49-40fcb487859c@oracle.com>
Date: Tue, 12 Sep 2017 11:41:19 -0400
From: Boris Ostrovsky <boris.ostrovsky@...cle.com>
To: Juergen Gross <jgross@...e.com>, linux-kernel@...r.kernel.org,
xen-devel@...ts.xenproject.org
Subject: Re: [Xen-devel] [PATCH 1/4] xen: re-introduce support for grant v2
interface
> +int arch_gnttab_map_status(uint64_t *frames, unsigned long nr_gframes,
> + unsigned long max_nr_gframes,
> + grant_status_t **__shared)
> +{
> + grant_status_t *shared = *__shared;
> + unsigned long addr;
> + unsigned long i;
> +
> + if (shared == NULL)
> + *__shared = shared = gnttab_status_vm_area.area->addr;
> +
> + addr = (unsigned long)shared;
> +
> + for (i = 0; i < nr_gframes; i++) {
> + set_pte_at(&init_mm, addr, gnttab_status_vm_area.ptes[i],
> + mfn_pte(frames[i], PAGE_KERNEL));
> + addr += PAGE_SIZE;
> + }
> +
> + return 0;
> +}
This looks pretty much identical to arch_gnttab_map_shared() except for
gnttab_shared_vm_area vs. gnttab_status_vm_area,which can be passed in
as a parameter.
> +
> void arch_gnttab_unmap(void *shared, unsigned long nr_gframes)
> {
> + pte_t **ptes;
> unsigned long addr;
> unsigned long i;
>
> + if (shared == gnttab_status_vm_area.area->addr)
> + ptes = gnttab_status_vm_area.ptes;
> + else
> + ptes = gnttab_shared_vm_area.ptes;
> +
> addr = (unsigned long)shared;
>
> for (i = 0; i < nr_gframes; i++) {
> - set_pte_at(&init_mm, addr, gnttab_shared_vm_area.ptes[i],
> - __pte(0));
> + set_pte_at(&init_mm, addr, ptes[i], __pte(0));
> addr += PAGE_SIZE;
> }
And this too looks like can be factored out (to something like
arch_update_gnttab()). But perhaps not in this patch.
> }
> @@ -102,12 +129,35 @@ static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned nr_frames)
> return 0;
> }
>
> -int arch_gnttab_init(unsigned long nr_shared)
> +static void arch_gnttab_vfree(struct gnttab_vm_area *area)
> {
> + free_vm_area(area->area);
> + kfree(area->ptes);
> +}
Not sure there is need for this routine. It is used only once.
Also, as an overall comment -- It feels like there may be too many
BUG_ON()s.
-boris
Powered by blists - more mailing lists