* [PATCH] x86: adjustments to memory_add()
@ 2015-08-28 13:54 Jan Beulich
2015-08-28 14:11 ` Andrew Cooper
2015-08-31 11:44 ` Wei Liu
0 siblings, 2 replies; 5+ messages in thread
From: Jan Beulich @ 2015-08-28 13:54 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser, Wei Liu
[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]
The function should clean up after a failed map_pages_to_xen().
Sharing the M2P table with Dom0 needs to happen before adding
the new pages to the heap.
Avoid the IOMMU mapping loop whenever possible.
Drop a redundant setting of 'ret'.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1376,7 +1376,7 @@ int memory_add(unsigned long spfn, unsig
ret = map_pages_to_xen((unsigned long)mfn_to_virt(spfn), spfn,
min(epfn, i) - spfn, PAGE_HYPERVISOR);
if ( ret )
- return ret;
+ goto destroy_directmap;
}
if ( i < epfn )
{
@@ -1385,7 +1385,7 @@ int memory_add(unsigned long spfn, unsig
ret = map_pages_to_xen((unsigned long)mfn_to_virt(i), i,
epfn - i, __PAGE_HYPERVISOR_RW);
if ( ret )
- return ret;
+ goto destroy_directmap;
}
old_node_start = NODE_DATA(node)->node_start_pfn;
@@ -1408,7 +1408,6 @@ int memory_add(unsigned long spfn, unsig
NODE_DATA(node)->node_spanned_pages = epfn - node_start_pfn(node);
}
- ret = -EINVAL;
info.spfn = spfn;
info.epfn = epfn;
info.cur = spfn;
@@ -1431,7 +1430,7 @@ int memory_add(unsigned long spfn, unsig
if ( ret )
goto destroy_m2p;
- if ( !need_iommu(hardware_domain) )
+ if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
@@ -1445,9 +1444,8 @@ int memory_add(unsigned long spfn, unsig
}
/* We can't revert any more */
- transfer_pages_to_heap(&info);
-
share_hotadd_m2p_table(&info);
+ transfer_pages_to_heap(&info);
return 0;
@@ -1458,13 +1456,13 @@ destroy_m2p:
max_pdx = pfn_to_pdx(max_page - 1) + 1;
destroy_frametable:
cleanup_frame_table(&info);
- destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
- (unsigned long)mfn_to_virt(epfn));
-
if ( !orig_online )
node_set_offline(node);
NODE_DATA(node)->node_start_pfn = old_node_start;
NODE_DATA(node)->node_spanned_pages = old_node_span;
+ destroy_directmap:
+ destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
+ (unsigned long)mfn_to_virt(epfn));
return ret;
}
[-- Attachment #2: x86-memadd-adjust.patch --]
[-- Type: text/plain, Size: 2564 bytes --]
x86: adjustments to memory_add()
The function should clean up after a failed map_pages_to_xen().
Sharing the M2P table with Dom0 needs to happen before adding
the new pages to the heap.
Avoid the IOMMU mapping loop whenever possible.
Drop a redundant setting of 'ret'.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1376,7 +1376,7 @@ int memory_add(unsigned long spfn, unsig
ret = map_pages_to_xen((unsigned long)mfn_to_virt(spfn), spfn,
min(epfn, i) - spfn, PAGE_HYPERVISOR);
if ( ret )
- return ret;
+ goto destroy_directmap;
}
if ( i < epfn )
{
@@ -1385,7 +1385,7 @@ int memory_add(unsigned long spfn, unsig
ret = map_pages_to_xen((unsigned long)mfn_to_virt(i), i,
epfn - i, __PAGE_HYPERVISOR_RW);
if ( ret )
- return ret;
+ goto destroy_directmap;
}
old_node_start = NODE_DATA(node)->node_start_pfn;
@@ -1408,7 +1408,6 @@ int memory_add(unsigned long spfn, unsig
NODE_DATA(node)->node_spanned_pages = epfn - node_start_pfn(node);
}
- ret = -EINVAL;
info.spfn = spfn;
info.epfn = epfn;
info.cur = spfn;
@@ -1431,7 +1430,7 @@ int memory_add(unsigned long spfn, unsig
if ( ret )
goto destroy_m2p;
- if ( !need_iommu(hardware_domain) )
+ if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
@@ -1445,9 +1444,8 @@ int memory_add(unsigned long spfn, unsig
}
/* We can't revert any more */
- transfer_pages_to_heap(&info);
-
share_hotadd_m2p_table(&info);
+ transfer_pages_to_heap(&info);
return 0;
@@ -1458,13 +1456,13 @@ destroy_m2p:
max_pdx = pfn_to_pdx(max_page - 1) + 1;
destroy_frametable:
cleanup_frame_table(&info);
- destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
- (unsigned long)mfn_to_virt(epfn));
-
if ( !orig_online )
node_set_offline(node);
NODE_DATA(node)->node_start_pfn = old_node_start;
NODE_DATA(node)->node_spanned_pages = old_node_span;
+ destroy_directmap:
+ destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
+ (unsigned long)mfn_to_virt(epfn));
return ret;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86: adjustments to memory_add()
2015-08-28 13:54 [PATCH] x86: adjustments to memory_add() Jan Beulich
@ 2015-08-28 14:11 ` Andrew Cooper
2015-08-28 14:29 ` Jan Beulich
2015-08-31 11:44 ` Wei Liu
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2015-08-28 14:11 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Wei Liu, Keir Fraser
[-- Attachment #1.1: Type: text/plain, Size: 2892 bytes --]
On 28/08/15 14:54, Jan Beulich wrote:
> The function should clean up after a failed map_pages_to_xen().
>
> Sharing the M2P table with Dom0 needs to happen before adding
> the new pages to the heap.
Why? Does this not create a race where dom0 can observe the new mfns
before they are ready to use?
~Andrew
>
> Avoid the IOMMU mapping loop whenever possible.
>
> Drop a redundant setting of 'ret'.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1376,7 +1376,7 @@ int memory_add(unsigned long spfn, unsig
> ret = map_pages_to_xen((unsigned long)mfn_to_virt(spfn), spfn,
> min(epfn, i) - spfn, PAGE_HYPERVISOR);
> if ( ret )
> - return ret;
> + goto destroy_directmap;
> }
> if ( i < epfn )
> {
> @@ -1385,7 +1385,7 @@ int memory_add(unsigned long spfn, unsig
> ret = map_pages_to_xen((unsigned long)mfn_to_virt(i), i,
> epfn - i, __PAGE_HYPERVISOR_RW);
> if ( ret )
> - return ret;
> + goto destroy_directmap;
> }
>
> old_node_start = NODE_DATA(node)->node_start_pfn;
> @@ -1408,7 +1408,6 @@ int memory_add(unsigned long spfn, unsig
> NODE_DATA(node)->node_spanned_pages = epfn - node_start_pfn(node);
> }
>
> - ret = -EINVAL;
> info.spfn = spfn;
> info.epfn = epfn;
> info.cur = spfn;
> @@ -1431,7 +1430,7 @@ int memory_add(unsigned long spfn, unsig
> if ( ret )
> goto destroy_m2p;
>
> - if ( !need_iommu(hardware_domain) )
> + if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
> {
> for ( i = spfn; i < epfn; i++ )
> if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
> @@ -1445,9 +1444,8 @@ int memory_add(unsigned long spfn, unsig
> }
>
> /* We can't revert any more */
> - transfer_pages_to_heap(&info);
> -
> share_hotadd_m2p_table(&info);
> + transfer_pages_to_heap(&info);
>
> return 0;
>
> @@ -1458,13 +1456,13 @@ destroy_m2p:
> max_pdx = pfn_to_pdx(max_page - 1) + 1;
> destroy_frametable:
> cleanup_frame_table(&info);
> - destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
> - (unsigned long)mfn_to_virt(epfn));
> -
> if ( !orig_online )
> node_set_offline(node);
> NODE_DATA(node)->node_start_pfn = old_node_start;
> NODE_DATA(node)->node_spanned_pages = old_node_span;
> + destroy_directmap:
> + destroy_xen_mappings((unsigned long)mfn_to_virt(spfn),
> + (unsigned long)mfn_to_virt(epfn));
>
> return ret;
> }
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3674 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86: adjustments to memory_add()
2015-08-28 14:11 ` Andrew Cooper
@ 2015-08-28 14:29 ` Jan Beulich
2015-08-28 14:31 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2015-08-28 14:29 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser, Wei Liu
>>> On 28.08.15 at 16:11, <andrew.cooper3@citrix.com> wrote:
> On 28/08/15 14:54, Jan Beulich wrote:
>> The function should clean up after a failed map_pages_to_xen().
>>
>> Sharing the M2P table with Dom0 needs to happen before adding
>> the new pages to the heap.
>
> Why? Does this not create a race where dom0 can observe the new mfns
> before they are ready to use?
No, observing the MFNs is fine as long as no domain can use them.
The race is the other way around: Not having the M2P in place
when the pages could already be handed out from the allocator is
a problem.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: adjustments to memory_add()
2015-08-28 14:29 ` Jan Beulich
@ 2015-08-28 14:31 ` Andrew Cooper
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2015-08-28 14:31 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Wei Liu
On 28/08/15 15:29, Jan Beulich wrote:
>>>> On 28.08.15 at 16:11, <andrew.cooper3@citrix.com> wrote:
>> On 28/08/15 14:54, Jan Beulich wrote:
>>> The function should clean up after a failed map_pages_to_xen().
>>>
>>> Sharing the M2P table with Dom0 needs to happen before adding
>>> the new pages to the heap.
>> Why? Does this not create a race where dom0 can observe the new mfns
>> before they are ready to use?
> No, observing the MFNs is fine as long as no domain can use them.
> The race is the other way around: Not having the M2P in place
> when the pages could already be handed out from the allocator is
> a problem.
Ah yes - of course. Would you mind extending the patch description to
include this.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: adjustments to memory_add()
2015-08-28 13:54 [PATCH] x86: adjustments to memory_add() Jan Beulich
2015-08-28 14:11 ` Andrew Cooper
@ 2015-08-31 11:44 ` Wei Liu
1 sibling, 0 replies; 5+ messages in thread
From: Wei Liu @ 2015-08-31 11:44 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Wei Liu, Andrew Cooper
On Fri, Aug 28, 2015 at 07:54:09AM -0600, Jan Beulich wrote:
> The function should clean up after a failed map_pages_to_xen().
>
> Sharing the M2P table with Dom0 needs to happen before adding
> the new pages to the heap.
>
> Avoid the IOMMU mapping loop whenever possible.
>
> Drop a redundant setting of 'ret'.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-31 11:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-28 13:54 [PATCH] x86: adjustments to memory_add() Jan Beulich
2015-08-28 14:11 ` Andrew Cooper
2015-08-28 14:29 ` Jan Beulich
2015-08-28 14:31 ` Andrew Cooper
2015-08-31 11:44 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.