* [PATCH] memory: don't suppress P2M update in populate_physmap()
@ 2017-06-20 6:19 Jan Beulich
2017-06-20 8:37 ` Andrew Cooper
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-06-20 6:19 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall
[-- Attachment #1: Type: text/plain, Size: 872 bytes --]
Commit d18627583d ("memory: don't hand MFN info to translated guests")
wrongly added a null-handle check there - just like stated in its
description for memory_exchange(), the array is also an input for
populate_physmap() (and hence can't reasonably be null). I have no idea
how I've managed to overlook this.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -248,8 +248,7 @@ static void populate_physmap(struct memo
guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), a->extent_order);
- if ( !paging_mode_translate(d) &&
- !guest_handle_is_null(a->extent_list) )
+ if ( !paging_mode_translate(d) )
{
for ( j = 0; j < (1U << a->extent_order); j++ )
set_gpfn_from_mfn(mfn + j, gpfn + j);
[-- Attachment #2: memory-populate-set-p2m.patch --]
[-- Type: text/plain, Size: 925 bytes --]
memory: don't suppress P2M update in populate_physmap()
Commit d18627583d ("memory: don't hand MFN info to translated guests")
wrongly added a null-handle check there - just like stated in its
description for memory_exchange(), the array is also an input for
populate_physmap() (and hence can't reasonably be null). I have no idea
how I've managed to overlook this.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -248,8 +248,7 @@ static void populate_physmap(struct memo
guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), a->extent_order);
- if ( !paging_mode_translate(d) &&
- !guest_handle_is_null(a->extent_list) )
+ if ( !paging_mode_translate(d) )
{
for ( j = 0; j < (1U << a->extent_order); j++ )
set_gpfn_from_mfn(mfn + j, gpfn + j);
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 6:19 [PATCH] memory: don't suppress P2M update in populate_physmap() Jan Beulich
@ 2017-06-20 8:37 ` Andrew Cooper
2017-06-20 12:39 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2017-06-20 8:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, Julien Grall
On 20/06/17 07:19, Jan Beulich wrote:
> Commit d18627583d ("memory: don't hand MFN info to translated guests")
> wrongly added a null-handle check there - just like stated in its
> description for memory_exchange(), the array is also an input for
> populate_physmap() (and hence can't reasonably be null). I have no idea
> how I've managed to overlook this.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 8:37 ` Andrew Cooper
@ 2017-06-20 12:39 ` Julien Grall
2017-06-20 12:40 ` Andrew Cooper
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2017-06-20 12:39 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson
Hi,
On 06/20/2017 09:37 AM, Andrew Cooper wrote:
> On 20/06/17 07:19, Jan Beulich wrote:
>> Commit d18627583d ("memory: don't hand MFN info to translated guests")
>> wrongly added a null-handle check there - just like stated in its
>> description for memory_exchange(), the array is also an input for
>> populate_physmap() (and hence can't reasonably be null). I have no idea
>> how I've managed to overlook this.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Am I correct that this is not a bug and only a pointless check?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 12:39 ` Julien Grall
@ 2017-06-20 12:40 ` Andrew Cooper
2017-06-20 12:51 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2017-06-20 12:40 UTC (permalink / raw)
To: Julien Grall, Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson
On 20/06/17 13:39, Julien Grall wrote:
> Hi,
>
> On 06/20/2017 09:37 AM, Andrew Cooper wrote:
>> On 20/06/17 07:19, Jan Beulich wrote:
>>> Commit d18627583d ("memory: don't hand MFN info to translated guests")
>>> wrongly added a null-handle check there - just like stated in its
>>> description for memory_exchange(), the array is also an input for
>>> populate_physmap() (and hence can't reasonably be null). I have no idea
>>> how I've managed to overlook this.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Am I correct that this is not a bug and only a pointless check?
This is a partial reversion of d18627583d and needs to be included in
4.9, to avoid a regression.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 12:40 ` Andrew Cooper
@ 2017-06-20 12:51 ` Julien Grall
2017-06-20 13:03 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2017-06-20 12:51 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson
Hi,
On 06/20/2017 01:40 PM, Andrew Cooper wrote:
> On 20/06/17 13:39, Julien Grall wrote:
>> Hi,
>>
>> On 06/20/2017 09:37 AM, Andrew Cooper wrote:
>>> On 20/06/17 07:19, Jan Beulich wrote:
>>>> Commit d18627583d ("memory: don't hand MFN info to translated guests")
>>>> wrongly added a null-handle check there - just like stated in its
>>>> description for memory_exchange(), the array is also an input for
>>>> populate_physmap() (and hence can't reasonably be null). I have no idea
>>>> how I've managed to overlook this.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> Am I correct that this is not a bug and only a pointless check?
>
> This is a partial reversion of d18627583d and needs to be included in
> 4.9, to avoid a regression.
Would you mind to explain why this would introduce regression? AFAICT
the check is just redundant, so keeping it is not that bad.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 12:51 ` Julien Grall
@ 2017-06-20 13:03 ` Jan Beulich
2017-06-20 14:05 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-06-20 13:03 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
>>> On 20.06.17 at 14:51, <julien.grall@arm.com> wrote:
> On 06/20/2017 01:40 PM, Andrew Cooper wrote:
>> On 20/06/17 13:39, Julien Grall wrote:
>>> On 06/20/2017 09:37 AM, Andrew Cooper wrote:
>>>> On 20/06/17 07:19, Jan Beulich wrote:
>>>>> Commit d18627583d ("memory: don't hand MFN info to translated guests")
>>>>> wrongly added a null-handle check there - just like stated in its
>>>>> description for memory_exchange(), the array is also an input for
>>>>> populate_physmap() (and hence can't reasonably be null). I have no idea
>>>>> how I've managed to overlook this.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> Am I correct that this is not a bug and only a pointless check?
>>
>> This is a partial reversion of d18627583d and needs to be included in
>> 4.9, to avoid a regression.
>
> Would you mind to explain why this would introduce regression? AFAICT
> the check is just redundant, so keeping it is not that bad.
Afaict there would be a regression only if someone invoked the
hypercall with a null handle (but having valid data at address zero).
Still I agree with Andrew that we'd better include this in 4.9.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] memory: don't suppress P2M update in populate_physmap()
2017-06-20 13:03 ` Jan Beulich
@ 2017-06-20 14:05 ` Julien Grall
0 siblings, 0 replies; 7+ messages in thread
From: Julien Grall @ 2017-06-20 14:05 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, xen-devel
On 06/20/2017 02:03 PM, Jan Beulich wrote:
>>>> On 20.06.17 at 14:51, <julien.grall@arm.com> wrote:
>> On 06/20/2017 01:40 PM, Andrew Cooper wrote:
>>> On 20/06/17 13:39, Julien Grall wrote:
>>>> On 06/20/2017 09:37 AM, Andrew Cooper wrote:
>>>>> On 20/06/17 07:19, Jan Beulich wrote:
>>>>>> Commit d18627583d ("memory: don't hand MFN info to translated guests")
>>>>>> wrongly added a null-handle check there - just like stated in its
>>>>>> description for memory_exchange(), the array is also an input for
>>>>>> populate_physmap() (and hence can't reasonably be null). I have no idea
>>>>>> how I've managed to overlook this.
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>
>>>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>
>>>> Am I correct that this is not a bug and only a pointless check?
>>>
>>> This is a partial reversion of d18627583d and needs to be included in
>>> 4.9, to avoid a regression.
>>
>> Would you mind to explain why this would introduce regression? AFAICT
>> the check is just redundant, so keeping it is not that bad.
>
> Afaict there would be a regression only if someone invoked the
> hypercall with a null handle (but having valid data at address zero).
> Still I agree with Andrew that we'd better include this in 4.9.
Hmmm. It didn't occur to me that someone would put valid data at address
zero.
Release-acked-by: Julien Grall <julien.grall@arm.com>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-20 14:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-20 6:19 [PATCH] memory: don't suppress P2M update in populate_physmap() Jan Beulich
2017-06-20 8:37 ` Andrew Cooper
2017-06-20 12:39 ` Julien Grall
2017-06-20 12:40 ` Andrew Cooper
2017-06-20 12:51 ` Julien Grall
2017-06-20 13:03 ` Jan Beulich
2017-06-20 14:05 ` Julien Grall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).