Re: [Xen-devel] [PATCH v21 1/2] common: add a new mappable resource type: XENMEM_resource_grant_table

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.08.18 at 12:46, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 08/08/18 11:43, Jan Beulich wrote:
>>>>> On 08.08.18 at 12:38, <Paul.Durrant@xxxxxxxxxx> wrote:
>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>> Sent: 08 August 2018 11:30
>>>>
>>>>>>> On 08.08.18 at 11:00, <paul.durrant@xxxxxxxxxx> wrote:
>>>>> +int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
>>>>> +                            mfn_t *mfn)
>>>>> +{
>>>>> +    struct grant_table *gt = d->grant_table;
>>>>> +    int rc;
>>>>> +
>>>>> +    grant_write_lock(gt);
>>>>> +
>>>>> +    if ( gt->gt_version == 0 )
>>>>> +        gt->gt_version = 1;
>>>> Since you've moved this here instead of dropping it, what requirement
>>>> have you found for this to be set (other than the ASSERT() you put in
>>>> gnttab_get_shared_frame_mfn()?
>>>>
>>> The code in patch #2 is executed before the grant table version is set. I 
>>> could alternatively have libxl explicitly set the version to 1 before 
>>> trying 
>>> to seed the table.
>> But that's not my point. What's wrong with leaving it at zero?
> 
> On a tangent, why does a gnttab version of 0 exist at all?  Why don't we
> explicitly initialise it to 1 in the hypervisor?

Fair question, which unfortunately I can't answer.

> We've had plenty of XSAs to do with mishandling of a gnttab version of
> 0.  Why not fix the problem at its source, and simplify the gnttab code
> while we are at it.

I don't mind, unless a reason for the seemingly strange behavior can be
found.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel