[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v1 21/25] hw/xen: Add emulated implementation of grant ta
From: |
David Woodhouse |
Subject: |
Re: [RFC PATCH v1 21/25] hw/xen: Add emulated implementation of grant table operations |
Date: |
Tue, 07 Mar 2023 16:16:06 +0000 |
User-agent: |
Evolution 3.44.4-0ubuntu1 |
On Tue, 2023-03-07 at 16:07 +0000, Paul Durrant wrote:
> On 02/03/2023 15:34, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> >
> > This is limited to mapping a single grant at a time, because under Xen the
> > pages are mapped *contiguously* into qemu's address space, and that's very
> > hard to do when those pages actually come from anonymous mappings in qemu
> > in the first place.
> >
> > Eventually perhaps we can look at using shared mappings of actual objects
> > for system RAM, and then we can make new mappings of the same backing
> > store (be it deleted files, shmem, whatever). But for now let's stick to
> > a page at a time.
> >
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> > hw/i386/kvm/xen_gnttab.c | 299 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 296 insertions(+), 3 deletions(-)
> >
> [snip]
> > +static uint64_t gnt_ref(XenGnttabState *s, grant_ref_t ref, int prot)
> > +{
> > + uint16_t mask = GTF_type_mask | GTF_sub_page;
> > + grant_entry_v1_t gnt, *gnt_p;
> > + int retries = 0;
> > +
> > + if (ref >= s->max_frames * ENTRIES_PER_FRAME_V1 ||
> > + s->map_track[ref] == UINT8_MAX) {
> > + return INVALID_GPA;
> > + }
> > +
> > + if (prot & PROT_WRITE) {
> > + mask |= GTF_readonly;
> > + }
> > +
> > + gnt_p = &s->entries.v1[ref];
> > +
> > + /*
> > + * The guest can legitimately be changing the GTF_readonly flag. Allow
>
> I'd call a guest playing with the ref after setting GTF_permit_access a
> buggy guest and not bother with the loop.
Didn't we have this argument already when I tried to get you to change
your one? :)
I argue that it's OK for a guest to *increase* permissions to include
write permission, even while a read-only grant may be in progress.
And also to *decrease* permissions to take away write permission, even
while read-only grants may be in progress.
The loop is what Xen does, so let's do it that way.
>
> > + * that, but don't let a malicious guest cause a livelock.
> > + */
> > + for (retries = 0; retries < 5; retries++) {
> > + uint16_t new_flags;
> > +
> > + /* Read the entry before an atomic operation on its flags */
> > + gnt = *(volatile grant_entry_v1_t *)gnt_p;
> > +
> > + if ((gnt.flags & mask) != GTF_permit_access ||
> > + gnt.domid != DOMID_QEMU) {
> > + return INVALID_GPA;
> > + }
> > +
> > + new_flags = gnt.flags | GTF_reading;
> > + if (prot & PROT_WRITE) {
> > + new_flags |= GTF_writing;
> > + }
> > +
> > + if (qatomic_cmpxchg(&gnt_p->flags, gnt.flags, new_flags) ==
> > gnt.flags) {
>
> Xen actually does a cmpxchg on both the flags and the domid. We probably
> ought to fail to set the flags if the guest is playing with the domid
> but since we're single-tenant it doesn't *really* matter... just a
> nice-to-have. So...
Yeah, changing the *domid* while it's active is definitely not an OK
thing to do.
smime.p7s
Description: S/MIME cryptographic signature
- Re: [RFC PATCH v1 03/25] hw/xen: Implement XenStore watches, (continued)
- [RFC PATCH v1 09/25] hw/xen: Add evtchn operations to allow redirection to internal emulation, David Woodhouse, 2023/03/02
- [RFC PATCH v1 16/25] hw/xen: Rename xen_common.h to xen_native.h, David Woodhouse, 2023/03/02
- [RFC PATCH v1 19/25] hw/xen: Only advertise ring-page-order for xen-block if gnttab supports it, David Woodhouse, 2023/03/02
- [RFC PATCH v1 21/25] hw/xen: Add emulated implementation of grant table operations, David Woodhouse, 2023/03/02
- [RFC PATCH v1 10/25] hw/xen: Add gnttab operations to allow redirection to internal emulation, David Woodhouse, 2023/03/02
- [RFC PATCH v1 23/25] hw/xen: Map guest XENSTORE_PFN grant in emulated Xenstore, David Woodhouse, 2023/03/02
- [RFC PATCH v1 25/25] i386/xen: Initialize Xen backends from pc_basic_device_init() for emulation, David Woodhouse, 2023/03/02
- [RFC PATCH v1 04/25] hw/xen: Implement XenStore transactions, David Woodhouse, 2023/03/02
- [RFC PATCH v1 06/25] hw/xen: Implement XenStore permissions, David Woodhouse, 2023/03/02