|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 05/24] xen: Preserve reserved grant entries when switching versions
It would be worth CCing the relevant maintainers for each patch in the
series. e.g. Keir in this case.
On Thu, 2012-01-26 at 19:44 +0000, Daniel De Graaf wrote:
> In order for the toolstack to use reserved grant table entries, the
> grant table for a guest must be initialized prior to the guest's boot.
> When the guest switches grant table versions (necessary if the guest is
> using v2 grant tables, or on kexec if switching grant versions), these
> initial grants will be cleared. Instead of clearing them, preserve
> the grants across the type change.
>
> Attempting to preserve v2-only features such as sub-page grants will
> produce a warning and invalidate the resulting v1 grant entry.
>
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> xen/common/grant_table.c | 48 +++++++++++++++++++++++++++++++++----
> xen/include/public/grant_table.h | 7 +++++
> 2 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 0c55fd1..6f24a94 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -2111,6 +2111,7 @@
> gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> struct domain *d = current->domain;
> struct grant_table *gt = d->grant_table;
> struct active_grant_entry *act;
> + grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
> long res;
> int i;
>
> @@ -2131,7 +2132,7 @@
> gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> /* (You need to change the version number for e.g. kexec.) */
> if ( gt->gt_version != 0 )
> {
> - for ( i = 0; i < nr_grant_entries(gt); i++ )
> + for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
The comment just prior says:
/* Make sure that the grant table isn't currently in use when we
change the version number. */
I think this needs updating to note that we do allow reserved entries to
be active during the switch over and we will correctly preserve
flags/status/mapped-ness etc.
> {
> act = &active_entry(gt, i);
> if ( act->pin != 0 )
> @@ -2156,15 +2157,50 @@
> gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> goto out_unlock;
> }
>
> + /* Preserve the first 8 entries (toolstack reserved grants) */
> + if (gt->gt_version == 1)
Xen coding style has extra spaces just inside the braces (and again
below a few more times).
> + {
> + memcpy(reserved_entries, gt->shared_v1[0], sizeof(reserved_entries));
Shouldn't that be either "gt->shared_v1" or ">->shared_v1[0]" ?
> + }
> + else if (gt->gt_version == 2)
> + {
> + for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i <
> nr_grant_entries(gt); i++ )
> + {
> + reserved_entries[i].flags = shared_entry_v2(gt, i).hdr.flags;
> + reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
> + reserved_entries[i].frame = shared_entry_v2(gt,
> i).full_page.frame;
> + reserved_entries[i].flags |= status_entry(gt, i);
> + if ((reserved_entries[i].flags & GTF_type_mask) >
> GTF_permit_access)
In effect this only allows GTF_permit_access or GTF_invalid, which is
good. It would be more obvious/explicit to do
if ((shared_entry_v2(gt, i).hdr.flags & GTF_type_mask) != GTF_invalid &&
(shared_entry_v2(gt, i).hdr.flags & GTF_type_mask) !=
GTF_permit_access)
memset-whole-entry and continue;
at the top or even a switch().
> + {
> + gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when
> switching grant version\n",
> + d->domain_id, reserved_entries[i].flags, i);
> + reserved_entries[i].flags = GTF_invalid;
> + }
> + }
> + }
> +
> if ( op.version < 2 && gt->gt_version == 2 )
> gnttab_unpopulate_status_frames(d, gt);
>
> - if ( op.version != gt->gt_version )
> + /* Make sure there's no crud left over in the table from the
> + old version. */
> + for ( i = 0; i < nr_grant_frames(gt); i++ )
> + memset(gt->shared_raw[i], 0, PAGE_SIZE);
> +
> + /* Restore the first 8 entries (toolstack reserved grants) */
> + if (gt->gt_version != 0 && op.version == 1)
> {
> - /* Make sure there's no crud left over in the table from the
> - old version. */
> - for ( i = 0; i < nr_grant_frames(gt); i++ )
> - memset(gt->shared_raw[i], 0, PAGE_SIZE);
> + memcpy(gt->shared_v1[0], reserved_entries, sizeof(reserved_entries));
> + }
> + else if (gt->gt_version != 0 && op.version == 2)
> + {
> + for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
> + {
> + status_entry(gt, i) = reserved_entries[i].flags &
> (GTF_reading|GTF_writing);
> + shared_entry_v2(gt, i).hdr.flags = reserved_entries[i].flags &
> ~(GTF_reading|GTF_writing);
> + shared_entry_v2(gt, i).hdr.domid = reserved_entries[i].domid;
> + shared_entry_v2(gt, i).full_page.frame =
> reserved_entries[i].frame;
> + }
> }
>
> gt->gt_version = op.version;
> diff --git a/xen/include/public/grant_table.h
> b/xen/include/public/grant_table.h
> index 54d9551..292d724 100644
> --- a/xen/include/public/grant_table.h
> +++ b/xen/include/public/grant_table.h
> @@ -117,6 +117,13 @@ struct grant_entry_v1 {
> };
> typedef struct grant_entry_v1 grant_entry_v1_t;
>
> +/* The first few grant table entries will be preserved across grant table
> + * version changes and may be pre-populated at domain creation by tools.
> + */
> +#define GNTTAB_NR_RESERVED_ENTRIES 8
> +#define GNTTAB_RESERVED_CONSOLE 0
> +#define GNTTAB_RESERVED_XENSTORE 1
> +
> /*
> * Type of grant entry.
> * GTF_invalid: This grant entry grants no privileges.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |