Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()

[Juergen Gross]

On 23/04/18 15:39, Andrew Cooper wrote:
> On 20/04/18 16:03, Jan Beulich wrote:
>> hvm_domain_initialise() may call this with nr being zero, which triggers
>> the "does not cross L3 boundary" check.
>>
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper 
> 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()

[Andrew Cooper]

On 20/04/18 16:03, Jan Beulich wrote:
> hvm_domain_initialise() may call this with nr being zero, which triggers
> the "does not cross L3 boundary" check.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()

[Jan Beulich]

>>> On 20.04.18 at 17:42,  wrote:
> On 20/04/18 16:27, Jan Beulich wrote:
> On 20.04.18 at 17:21,  wrote:
>>> On 20/04/18 16:03, Jan Beulich wrote:
 hvm_domain_initialise() may call this with nr being zero, which triggers
 the "does not cross L3 boundary" check.

 Signed-off-by: Jan Beulich 
>>> Is this the correct fix?
>>>
>>> Its unclear what the call to create_perdomain_mapping() is doing, but it
>>> is the sole caller which passes 0.
>>>
>>> I presume it is to allocate d->arch.perdomain_l3_pg but
>>> destroy_perdomain_mapping() doesn't free this page, so I've got no idea
>>> what the destroy call is trying to achieve.
>> Callers of this pair of functions should not be concerned about their
>> inner workings: If the create call succeeded, the destroy function should
>> be called for cleanup, even if that's _currently_ a no-op.
> 
> Right, but its still not clear that passing 0 (even to create) is a
> legitimate thing to do.

Well, the create function specifically handles the nr == 0 case, so it's
intended to be called that way (and, as you've said, exclusively for
setting up d->arch.perdomain_l3_pg).

Jan



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

Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()

[Andrew Cooper]

On 20/04/18 16:27, Jan Beulich wrote:
 On 20.04.18 at 17:21,  wrote:
>> On 20/04/18 16:03, Jan Beulich wrote:
>>> hvm_domain_initialise() may call this with nr being zero, which triggers
>>> the "does not cross L3 boundary" check.
>>>
>>> Signed-off-by: Jan Beulich 
>> Is this the correct fix?
>>
>> Its unclear what the call to create_perdomain_mapping() is doing, but it
>> is the sole caller which passes 0.
>>
>> I presume it is to allocate d->arch.perdomain_l3_pg but
>> destroy_perdomain_mapping() doesn't free this page, so I've got no idea
>> what the destroy call is trying to achieve.
> Callers of this pair of functions should not be concerned about their
> inner workings: If the create call succeeded, the destroy function should
> be called for cleanup, even if that's _currently_ a no-op.

Right, but its still not clear that passing 0 (even to create) is a
legitimate thing to do.

~Andrew

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

Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()

[Jan Beulich]

>>> On 20.04.18 at 17:21,  wrote:
> On 20/04/18 16:03, Jan Beulich wrote:
>> hvm_domain_initialise() may call this with nr being zero, which triggers
>> the "does not cross L3 boundary" check.
>>
>> Signed-off-by: Jan Beulich 
> 
> Is this the correct fix?
> 
> Its unclear what the call to create_perdomain_mapping() is doing, but it
> is the sole caller which passes 0.
> 
> I presume it is to allocate d->arch.perdomain_l3_pg but
> destroy_perdomain_mapping() doesn't free this page, so I've got no idea
> what the destroy call is trying to achieve.

Callers of this pair of functions should not be concerned about their
inner workings: If the create call succeeded, the destroy function should
be called for cleanup, even if that's _currently_ a no-op.

Jan



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