Re: Re: [PATCH] xen: reduce severity of message about using v1 grant tables.

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 12/03/09 04:01, Steven Smith wrote:
>> Oh, I see what you meant... in the proper resume case (as opposed to the
>> cancelled suspend/checkpoint case I was thinking of) there should be no
>> grant tables in use at this point so most devices should, in theory, be
>> able to reconnect using v1 grants, any drivers which require v2 grant
>> tables need to check for them in their resume hook as well as at start
>> of day.
>>
>> Unfortunately frontend devices tear down their grant entries after the
>> resume rather than before the suspend (I presume this has to do with
>> faster checkpointing?) which means they could be trying to clear an
>> entry of the wrong layout, leading the unbounded badness that the
>> comment refers to.
>>     
> Actually, I think that'd be okay.  Drivers should be clearing grant
> table entries through the gnttab_end_* functions, which always check
> grant_table_version and do the right thing.
> gnttab_grant_foreign_access_ref_{subpage,trans} would BUG(), but that
> could be turned into an error return easily enough.  Handling it
> everywhere would be a pain, though.
>
> The only other potential subtlety is handling a suspend while some
> other vcpu is doing grant table operations, but I think the
> stop_machine() is sufficient protection against that.
>   

Yes.  We already have to be careful to prevent pte updates from being
preempted by suspend, so grant operations are similar.  (When
CONFIG_PREEMPT is enabled, it is freeze/thaw which provides this guarantee.)

>  	BUG_ON(flags & (GTF_accept_transfer | GTF_reading |
>  			GTF_writing | GTF_sub_page | GTF_permit_access));
> -	BUG_ON(grant_table_version == 1);
> +	if (grant_table_version == 1)
>   

Rather than having all these version tests, would it make sense to have
a grant_ops vector?

    J