From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Andrew Cooper <amc96@cam.ac.uk>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
"Ian Jackson" <iwj@xenproject.org>,
"Michał Leszczyński" <michal.leszczynski@cert.pl>,
"Hubert Jasudowicz" <hubert.jasudowicz@cert.pl>,
"Tamas K Lengyel" <tamas@tklengyel.com>
Subject: Re: [PATCH v2 05/11] tools/foreignmem: Support querying the size of a resource
Date: Mon, 11 Jan 2021 11:50:01 +0100 [thread overview]
Message-ID: <20210111105001.xioy763vaior5udg@Air-de-Roger> (raw)
In-Reply-To: <22be067c-1e6f-c87d-f910-ced27091a87a@cam.ac.uk>
On Fri, Jan 08, 2021 at 05:52:36PM +0000, Andrew Cooper wrote:
> On 22/09/2020 19:24, Andrew Cooper wrote:
> > diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
> > index fe73d5ab72..eec089e232 100644
> > --- a/tools/libs/foreignmemory/linux.c
> > +++ b/tools/libs/foreignmemory/linux.c
> > @@ -339,6 +342,39 @@ int osdep_xenforeignmemory_map_resource(
> > return 0;
> > }
> >
> > +int osdep_xenforeignmemory_resource_size(
> > + xenforeignmemory_handle *fmem, domid_t domid, unsigned int type,
> > + unsigned int id, unsigned long *nr_frames)
> > +{
> > + int rc;
> > + struct xen_mem_acquire_resource *xmar =
> > + xencall_alloc_buffer(fmem->xcall, sizeof(*xmar));
> > +
> > + if ( !xmar )
> > + {
> > + PERROR("Could not bounce memory for acquire_resource hypercall");
> > + return -1;
> > + }
> > +
> > + *xmar = (struct xen_mem_acquire_resource){
> > + .domid = domid,
> > + .type = type,
> > + .id = id,
> > + };
> > +
> > + rc = xencall2(fmem->xcall, __HYPERVISOR_memory_op,
> > + XENMEM_acquire_resource, (uintptr_t)xmar);
> > + if ( rc )
> > + goto out;
> > +
> > + *nr_frames = xmar->nr_frames;
> > +
> > + out:
> > + xencall_free_buffer(fmem->xcall, xmar);
> > +
> > + return rc;
> > +}
>
> Having talked this through with Roger, it's broken.
>
> In the meantime, foreignmem has gained acquire_resource on FreeBSD.
> Nothing in this osdep function is linux-specific, so it oughtn't to be
> osdep.
>
> However, its also not permitted to make hypercalls like this in
> restricted mode, and that isn't something we should be breaking.
> Amongst other things, it will prevent us from supporting >128 cpus, as
> Qemu needs updating to use this interface in due course.
>
> The only solution (which keeps restricted mode working) is to fix
> Linux's ioctl() to be able to understand size requests. This also
> avoids foreignmem needing to open a xencall handle which was fugly in
> the first place.
I think the following patch should allow you to fetch the resource
size from Linux privcmd driver by doing an ioctl with addr = 0 and num
= 0. I've just build tested it, but I haven't tried exercising the
code.
Roger.
---8<---
From 5d717c7b9ad3561ed0b17e7c5cf76b7c9fb536db Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Mon, 11 Jan 2021 10:38:59 +0100
Subject: [PATCH] xen/privcmd: allow fetching resource sizes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Allow issuing an IOCTL_PRIVCMD_MMAP_RESOURCE ioctl with num = 0 and
addr = 0 in order to fetch the size of a specific resource.
Add a shortcut to the default map resource path, since fetching the
size requires no address to be passed in, and thus no VMA to setup.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
NB: fetching the size of a resource shouldn't trigger an hypercall
preemption, and hence I've dropped the preempt indications.
---
drivers/xen/privcmd.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index b0c73c58f987..3278f93eb3da 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -717,7 +717,8 @@ static long privcmd_ioctl_restrict(struct file *file, void __user *udata)
return 0;
}
-static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata)
+static long privcmd_ioctl_mmap_resource(struct file *file,
+ struct privcmd_mmap_resource __user *udata)
{
struct privcmd_data *data = file->private_data;
struct mm_struct *mm = current->mm;
@@ -734,6 +735,19 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata)
if (data->domid != DOMID_INVALID && data->domid != kdata.dom)
return -EPERM;
+ memset(&xdata, 0, sizeof(xdata));
+
+ if (!kdata.addr && !kdata.num) {
+ /* Query the size of the resource. */
+ xdata.domid = kdata.dom;
+ xdata.type = kdata.type;
+ xdata.id = kdata.id;
+ rc = HYPERVISOR_memory_op(XENMEM_acquire_resource, &xdata);
+ if (rc)
+ return rc;
+ return __put_user(xdata.nr_frames, &udata->num);
+ }
+
mmap_write_lock(mm);
vma = find_vma(mm, kdata.addr);
@@ -768,7 +782,6 @@ static long privcmd_ioctl_mmap_resource(struct file *file, void __user *udata)
} else
vma->vm_private_data = PRIV_VMA_LOCKED;
- memset(&xdata, 0, sizeof(xdata));
xdata.domid = kdata.dom;
xdata.type = kdata.type;
xdata.id = kdata.id;
--
2.29.2
next prev parent reply other threads:[~2021-01-11 10:50 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 18:24 [PATCH v2 00/11] Multiple fixes to XENMEM_acquire_resource Andrew Cooper
2020-09-22 18:24 ` [PATCH v2 01/11] xen/memory: Introduce CONFIG_ARCH_ACQUIRE_RESOURCE Andrew Cooper
2020-09-22 18:24 ` [PATCH v2 02/11] xen/gnttab: Rework resource acquisition Andrew Cooper
2020-09-24 9:51 ` Paul Durrant
2021-01-11 21:22 ` Andrew Cooper
2021-01-12 8:23 ` Jan Beulich
2021-01-12 20:06 ` Andrew Cooper
2021-01-12 8:29 ` Paul Durrant
2020-09-25 13:17 ` Jan Beulich
2021-01-11 21:22 ` Andrew Cooper
2021-01-12 8:15 ` Jan Beulich
2021-01-12 18:11 ` Andrew Cooper
2020-09-22 18:24 ` [PATCH v2 03/11] xen/memory: Fix compat XENMEM_acquire_resource for size requests Andrew Cooper
2020-09-22 18:24 ` [PATCH v2 04/11] xen/memory: Fix acquire_resource size semantics Andrew Cooper
2020-09-24 10:06 ` Paul Durrant
2020-09-24 10:57 ` Andrew Cooper
2020-09-24 11:04 ` Paul Durrant
2020-09-25 15:56 ` Jan Beulich
2020-09-22 18:24 ` [PATCH v2 05/11] tools/foreignmem: Support querying the size of a resource Andrew Cooper
2021-01-08 17:52 ` Andrew Cooper
2021-01-11 10:50 ` Roger Pau Monné [this message]
2021-01-11 15:00 ` Andrew Cooper
2021-01-11 15:26 ` [PATCH v3 " Andrew Cooper
2021-01-11 15:54 ` Roger Pau Monné
2020-09-22 18:24 ` [PATCH v2 06/11] xen/memory: Clarify the XENMEM_acquire_resource ABI description Andrew Cooper
2020-09-24 10:08 ` Paul Durrant
2020-09-22 18:24 ` [PATCH v2 07/11] xen/memory: Improve compat XENMEM_acquire_resource handling Andrew Cooper
2020-09-24 10:16 ` Paul Durrant
2020-09-28 9:09 ` Jan Beulich
2021-01-08 18:57 ` Andrew Cooper
2021-01-11 14:25 ` Jan Beulich
2020-09-22 18:24 ` [PATCH v2 08/11] xen/memory: Indent part of acquire_resource() Andrew Cooper
2020-09-24 10:36 ` Paul Durrant
2020-09-22 18:24 ` [PATCH v2 09/11] xen/memory: Fix mapping grant tables with XENMEM_acquire_resource Andrew Cooper
2020-09-24 10:47 ` Paul Durrant
2021-01-08 19:36 ` Andrew Cooper
2020-09-28 9:37 ` Jan Beulich
2021-01-11 20:05 ` Andrew Cooper
2021-01-11 22:36 ` Andrew Cooper
2021-01-12 8:39 ` Jan Beulich
2020-09-22 18:24 ` [PATCH v2 10/11] TESTING dom0 Andrew Cooper
2020-09-22 18:24 ` [PATCH v2 11/11] TESTING XTF Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210111105001.xioy763vaior5udg@Air-de-Roger \
--to=roger.pau@citrix.com \
--cc=amc96@cam.ac.uk \
--cc=hubert.jasudowicz@cert.pl \
--cc=iwj@xenproject.org \
--cc=michal.leszczynski@cert.pl \
--cc=tamas@tklengyel.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.