* [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
2016-12-05 16:34 [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Juergen Gross
@ 2016-12-05 16:34 ` Juergen Gross
2016-12-05 17:17 ` Jan Beulich
[not found] ` <5845AF3E020000780012541F@suse.com>
2016-12-05 16:34 ` [PATCH 2/2] xen/x86: add a way to obtain the needed number of memory map entries Juergen Gross
2016-12-05 16:39 ` [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Andrew Cooper
2 siblings, 2 replies; 13+ messages in thread
From: Juergen Gross @ 2016-12-05 16:34 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
andrew.cooper3, ian.jackson, tim, jbeulich
For XENMEM_machine_memory_map the hypervisor returns EINVAL if the
caller's buffer can't hold all entries.
This is a problem as the caller has normally a static buffer defined
and when he is doing the call no dynamic memory allocation is
possible as nothing is yet known about the system's memory layout.
Instead of just fail deliver as many memory map entries as possible
and return with E2BIG indicating the result was incomplete. Then the
caller will be capable to use at least some memory reported to exist
to allocate a larger buffer for the complete memory map.
As E2BIG wasn't returned before a caller not prepared for this case
will still see just a failure as before, while someone prepared for
this error code running on an old hypervisor won't run into problems
other than without this change.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/mm.c | 22 ++++++++++++----------
xen/include/public/memory.h | 2 ++
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 14552a1..f8e679d 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4737,7 +4737,7 @@ static int _handle_iomem_range(unsigned long s, unsigned long e,
XEN_GUEST_HANDLE(e820entry_t) buffer;
if ( ctxt->n + 1 >= ctxt->map.nr_entries )
- return -EINVAL;
+ return -E2BIG;
ent.addr = (uint64_t)ctxt->s << PAGE_SHIFT;
ent.size = (uint64_t)(s - ctxt->s) << PAGE_SHIFT;
ent.type = E820_RESERVED;
@@ -4985,8 +4985,6 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( copy_from_guest(&ctxt.map, arg, 1) )
return -EFAULT;
- if ( ctxt.map.nr_entries < e820.nr_map + 1 )
- return -EINVAL;
buffer_param = guest_handle_cast(ctxt.map.buffer, e820entry_t);
buffer = guest_handle_from_param(buffer_param, e820entry_t);
@@ -5005,31 +5003,35 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( !rc )
rc = handle_iomem_range(s, s, &ctxt);
if ( rc )
- return rc;
+ break;
+ }
+ if ( ctxt.map.nr_entries <= ctxt.n + 1 )
+ {
+ rc = -E2BIG;
+ break;
}
- if ( ctxt.map.nr_entries <= ctxt.n + (e820.nr_map - i) )
- return -EINVAL;
if ( __copy_to_guest_offset(buffer, ctxt.n, e820.map + i, 1) )
return -EFAULT;
ctxt.s = PFN_UP(e820.map[i].addr + e820.map[i].size);
}
- if ( ctxt.s )
+ if ( !rc && ctxt.s )
{
rc = rangeset_report_ranges(current->domain->iomem_caps, ctxt.s,
~0UL, handle_iomem_range, &ctxt);
if ( !rc && ctxt.s )
rc = handle_iomem_range(~0UL, ~0UL, &ctxt);
- if ( rc )
- return rc;
}
+ if ( rc && rc != -E2BIG )
+ return rc;
+
ctxt.map.nr_entries = ctxt.n;
if ( __copy_to_guest(arg, &ctxt.map, 1) )
return -EFAULT;
- return 0;
+ return rc;
}
case XENMEM_machphys_mapping:
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5bf840f..20df769 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -339,6 +339,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_memory_map_t);
/*
* Returns the real physical memory map. Passes the same structure as
* XENMEM_memory_map.
+ * In case of a buffer not capable to hold all entries of the physical
+ * memory map -E2BIG is returned and the buffer is filled completely.
* arg == addr of xen_memory_map_t.
*/
#define XENMEM_machine_memory_map 10
--
2.10.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
2016-12-05 16:34 ` [PATCH 1/2] xen/x86: return partial memory map in case of not enough space Juergen Gross
@ 2016-12-05 17:17 ` Jan Beulich
[not found] ` <5845AF3E020000780012541F@suse.com>
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-12-05 17:17 UTC (permalink / raw)
To: xen-devel, Juergen Gross
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim
>>> On 05.12.16 at 17:34, <JGross@suse.com> wrote:
> For XENMEM_machine_memory_map the hypervisor returns EINVAL if the
> caller's buffer can't hold all entries.
>
> This is a problem as the caller has normally a static buffer defined
> and when he is doing the call no dynamic memory allocation is
> possible as nothing is yet known about the system's memory layout.
>
> Instead of just fail deliver as many memory map entries as possible
> and return with E2BIG indicating the result was incomplete. Then the
> caller will be capable to use at least some memory reported to exist
> to allocate a larger buffer for the complete memory map.
This makes no sense, as what we're talking about here is the
machine memory map, and the calling Dom0 kernel can't allocate
from that pool directly. Instead it would need its own memory
map to know where to place such a larger buffer, and this map
is usually just one or two entries large.
For that reason I'm not convinced we need or want this change.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread[parent not found: <5845AF3E020000780012541F@suse.com>]
* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
[not found] ` <5845AF3E020000780012541F@suse.com>
@ 2016-12-06 7:43 ` Juergen Gross
2016-12-06 8:15 ` Jan Beulich
[not found] ` <584681CA020000780012577A@suse.com>
0 siblings, 2 replies; 13+ messages in thread
From: Juergen Gross @ 2016-12-06 7:43 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim
On 05/12/16 18:17, Jan Beulich wrote:
>>>> On 05.12.16 at 17:34, <JGross@suse.com> wrote:
>> For XENMEM_machine_memory_map the hypervisor returns EINVAL if the
>> caller's buffer can't hold all entries.
>>
>> This is a problem as the caller has normally a static buffer defined
>> and when he is doing the call no dynamic memory allocation is
>> possible as nothing is yet known about the system's memory layout.
>>
>> Instead of just fail deliver as many memory map entries as possible
>> and return with E2BIG indicating the result was incomplete. Then the
>> caller will be capable to use at least some memory reported to exist
>> to allocate a larger buffer for the complete memory map.
>
> This makes no sense, as what we're talking about here is the
> machine memory map, and the calling Dom0 kernel can't allocate
> from that pool directly. Instead it would need its own memory
> map to know where to place such a larger buffer, and this map
> is usually just one or two entries large.
This is true. In practice, however, things are a little bit more
complicated:
Linux being started as dom0 tries to rearrange memory layout to match
the one of the physical machine. It will only add memory to its
allocator which is known to either not need to be moved or which has
already been moved. And this decision is based on the machine memory
map.
I admit it is a Linux kernel private decision how to handle the boot and
adding of memory to its allocator. OTOH the "all or nothing" approach of
the hypervisor regarding delivery of the machine memory map is a little
bit strange, especially as the BIOS is returning the E820 map one entry
after the other.
> For that reason I'm not convinced we need or want this change.
It would certainly make it easier for the Linux kernel.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
2016-12-06 7:43 ` Juergen Gross
@ 2016-12-06 8:15 ` Jan Beulich
[not found] ` <584681CA020000780012577A@suse.com>
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-12-06 8:15 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim, xen-devel
>>> On 06.12.16 at 08:43, <JGross@suse.com> wrote:
> On 05/12/16 18:17, Jan Beulich wrote:
>>>>> On 05.12.16 at 17:34, <JGross@suse.com> wrote:
>>> For XENMEM_machine_memory_map the hypervisor returns EINVAL if the
>>> caller's buffer can't hold all entries.
>>>
>>> This is a problem as the caller has normally a static buffer defined
>>> and when he is doing the call no dynamic memory allocation is
>>> possible as nothing is yet known about the system's memory layout.
>>>
>>> Instead of just fail deliver as many memory map entries as possible
>>> and return with E2BIG indicating the result was incomplete. Then the
>>> caller will be capable to use at least some memory reported to exist
>>> to allocate a larger buffer for the complete memory map.
>>
>> This makes no sense, as what we're talking about here is the
>> machine memory map, and the calling Dom0 kernel can't allocate
>> from that pool directly. Instead it would need its own memory
>> map to know where to place such a larger buffer, and this map
>> is usually just one or two entries large.
>
> This is true. In practice, however, things are a little bit more
> complicated:
>
> Linux being started as dom0 tries to rearrange memory layout to match
> the one of the physical machine. It will only add memory to its
> allocator which is known to either not need to be moved or which has
> already been moved. And this decision is based on the machine memory
> map.
Right, I did recall this oddity only after having sent the initial reply
(I'm still much more into how the non-pvops kernel works, and there's
no such dependency there).
> I admit it is a Linux kernel private decision how to handle the boot and
> adding of memory to its allocator. OTOH the "all or nothing" approach of
> the hypervisor regarding delivery of the machine memory map is a little
> bit strange, especially as the BIOS is returning the E820 map one entry
> after the other.
>
>> For that reason I'm not convinced we need or want this change.
>
> It would certainly make it easier for the Linux kernel.
I'd like us to at least consider alternatives:
1) Have a new sub-op behaving BIOS like (one entry at a time).
2) Make the full map available inside the initial mapping, pointed to
by a new entry in the start info structure.
3) Have pvops Linux make use of the extra space available at the
end of the initial mapping. The minimum of 512k there should be
more than enough.
4) Others I can't think of right now.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread[parent not found: <584681CA020000780012577A@suse.com>]
* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
[not found] ` <584681CA020000780012577A@suse.com>
@ 2016-12-06 8:33 ` Juergen Gross
2016-12-06 8:51 ` Jan Beulich
[not found] ` <58468A1202000078001257BE@suse.com>
0 siblings, 2 replies; 13+ messages in thread
From: Juergen Gross @ 2016-12-06 8:33 UTC (permalink / raw)
To: Jan Beulich
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim, xen-devel
On 06/12/16 09:15, Jan Beulich wrote:
>>>> On 06.12.16 at 08:43, <JGross@suse.com> wrote:
>> On 05/12/16 18:17, Jan Beulich wrote:
>>>>>> On 05.12.16 at 17:34, <JGross@suse.com> wrote:
>>>> For XENMEM_machine_memory_map the hypervisor returns EINVAL if the
>>>> caller's buffer can't hold all entries.
>>>>
>>>> This is a problem as the caller has normally a static buffer defined
>>>> and when he is doing the call no dynamic memory allocation is
>>>> possible as nothing is yet known about the system's memory layout.
>>>>
>>>> Instead of just fail deliver as many memory map entries as possible
>>>> and return with E2BIG indicating the result was incomplete. Then the
>>>> caller will be capable to use at least some memory reported to exist
>>>> to allocate a larger buffer for the complete memory map.
>>>
>>> This makes no sense, as what we're talking about here is the
>>> machine memory map, and the calling Dom0 kernel can't allocate
>>> from that pool directly. Instead it would need its own memory
>>> map to know where to place such a larger buffer, and this map
>>> is usually just one or two entries large.
>>
>> This is true. In practice, however, things are a little bit more
>> complicated:
>>
>> Linux being started as dom0 tries to rearrange memory layout to match
>> the one of the physical machine. It will only add memory to its
>> allocator which is known to either not need to be moved or which has
>> already been moved. And this decision is based on the machine memory
>> map.
>
> Right, I did recall this oddity only after having sent the initial reply
> (I'm still much more into how the non-pvops kernel works, and there's
> no such dependency there).
>
>> I admit it is a Linux kernel private decision how to handle the boot and
>> adding of memory to its allocator. OTOH the "all or nothing" approach of
>> the hypervisor regarding delivery of the machine memory map is a little
>> bit strange, especially as the BIOS is returning the E820 map one entry
>> after the other.
>>
>>> For that reason I'm not convinced we need or want this change.
>>
>> It would certainly make it easier for the Linux kernel.
>
> I'd like us to at least consider alternatives:
>
> 1) Have a new sub-op behaving BIOS like (one entry at a time).
No objections from me.
> 2) Make the full map available inside the initial mapping, pointed to
> by a new entry in the start info structure.
What about PVH?
> 3) Have pvops Linux make use of the extra space available at the
> end of the initial mapping. The minimum of 512k there should be
> more than enough.
This would mean using this area as a temporary buffer and copying the
obtained map somewhere else (either only in the size of the static
buffer or after reorganizing the memory according to the map). I guess
this would work, too.
> 4) Others I can't think of right now.
My preference would be 1). In case we are choosing this approach I
don't see any need for the other patch I sent for getting the number
of entries. It can easily be replaced by a loop of the single entry
solution.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
2016-12-06 8:33 ` Juergen Gross
@ 2016-12-06 8:51 ` Jan Beulich
[not found] ` <58468A1202000078001257BE@suse.com>
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-12-06 8:51 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim, xen-devel
>>> On 06.12.16 at 09:33, <JGross@suse.com> wrote:
> On 06/12/16 09:15, Jan Beulich wrote:
>> I'd like us to at least consider alternatives:
>>
>> 1) Have a new sub-op behaving BIOS like (one entry at a time).
>
> No objections from me.
>
>> 2) Make the full map available inside the initial mapping, pointed to
>> by a new entry in the start info structure.
>
> What about PVH?
Does PVH re-arrangement of its physical memory too? I didn't think
so ...
>> 3) Have pvops Linux make use of the extra space available at the
>> end of the initial mapping. The minimum of 512k there should be
>> more than enough.
>
> This would mean using this area as a temporary buffer and copying the
> obtained map somewhere else (either only in the size of the static
> buffer or after reorganizing the memory according to the map). I guess
> this would work, too.
>
>> 4) Others I can't think of right now.
>
> My preference would be 1). In case we are choosing this approach I
> don't see any need for the other patch I sent for getting the number
> of entries. It can easily be replaced by a loop of the single entry
> solution.
Well, my preference would be 3), then 2), then 1).
Regardless of that I would welcome patch 2 to be re-submitted
without depending on patch 1, as I think this is a worthwhile
adjustment in any case (bringing the interface in line with what
we do elsewhere).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread[parent not found: <58468A1202000078001257BE@suse.com>]
* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
[not found] ` <58468A1202000078001257BE@suse.com>
@ 2016-12-06 9:44 ` Juergen Gross
2016-12-06 9:51 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Juergen Gross @ 2016-12-06 9:44 UTC (permalink / raw)
To: Jan Beulich
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim, xen-devel
On 06/12/16 09:51, Jan Beulich wrote:
>>>> On 06.12.16 at 09:33, <JGross@suse.com> wrote:
>> On 06/12/16 09:15, Jan Beulich wrote:
>>> I'd like us to at least consider alternatives:
>>>
>>> 1) Have a new sub-op behaving BIOS like (one entry at a time).
>>
>> No objections from me.
>>
>>> 2) Make the full map available inside the initial mapping, pointed to
>>> by a new entry in the start info structure.
>>
>> What about PVH?
>
> Does PVH re-arrangement of its physical memory too? I didn't think
> so ...
Not yet. Are you sure it won't be needed in future? Why was it done in
the pvops kernel? I thought the main reason was to have the same PCI
space address layout as on the native system in order to avoid any bad
surprises due to buggy firmware and/or BIOS. The same could apply for
PVH.
>>> 3) Have pvops Linux make use of the extra space available at the
>>> end of the initial mapping. The minimum of 512k there should be
>>> more than enough.
>>
>> This would mean using this area as a temporary buffer and copying the
>> obtained map somewhere else (either only in the size of the static
>> buffer or after reorganizing the memory according to the map). I guess
>> this would work, too.
>>
>>> 4) Others I can't think of right now.
>>
>> My preference would be 1). In case we are choosing this approach I
>> don't see any need for the other patch I sent for getting the number
>> of entries. It can easily be replaced by a loop of the single entry
>> solution.
>
> Well, my preference would be 3), then 2), then 1).
If nobody is objecting I can go that route (3).
> Regardless of that I would welcome patch 2 to be re-submitted
> without depending on patch 1, as I think this is a worthwhile
> adjustment in any case (bringing the interface in line with what
> we do elsewhere).
Sure, will do.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/2] xen/x86: return partial memory map in case of not enough space
2016-12-06 9:44 ` Juergen Gross
@ 2016-12-06 9:51 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2016-12-06 9:51 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
tim, xen-devel
>>> On 06.12.16 at 10:44, <JGross@suse.com> wrote:
> On 06/12/16 09:51, Jan Beulich wrote:
>>>>> On 06.12.16 at 09:33, <JGross@suse.com> wrote:
>>> On 06/12/16 09:15, Jan Beulich wrote:
>>>> I'd like us to at least consider alternatives:
>>>>
>>>> 1) Have a new sub-op behaving BIOS like (one entry at a time).
>>>
>>> No objections from me.
>>>
>>>> 2) Make the full map available inside the initial mapping, pointed to
>>>> by a new entry in the start info structure.
>>>
>>> What about PVH?
>>
>> Does PVH re-arrangement of its physical memory too? I didn't think
>> so ...
>
> Not yet. Are you sure it won't be needed in future? Why was it done in
> the pvops kernel? I thought the main reason was to have the same PCI
> space address layout as on the native system in order to avoid any bad
> surprises due to buggy firmware and/or BIOS. The same could apply for
> PVH.
PVH Dom0 (iirc) gets its memory arranged around MMIO holes
already (by the hypervisor).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] xen/x86: add a way to obtain the needed number of memory map entries
2016-12-05 16:34 [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Juergen Gross
2016-12-05 16:34 ` [PATCH 1/2] xen/x86: return partial memory map in case of not enough space Juergen Gross
@ 2016-12-05 16:34 ` Juergen Gross
2016-12-05 16:39 ` [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Andrew Cooper
2 siblings, 0 replies; 13+ messages in thread
From: Juergen Gross @ 2016-12-05 16:34 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
andrew.cooper3, ian.jackson, tim, jbeulich
Today there is no way for a domain to obtain the number of entries of
the machine memory map returned by XENMEM_machine_memory_map hypercall.
Modify the interface to return just the needed number of map entries
in case the buffer was specified as NULL.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/mm.c | 38 +++++++++++++++++++++++---------------
xen/include/public/memory.h | 2 ++
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f8e679d..d384022 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4736,15 +4736,18 @@ static int _handle_iomem_range(unsigned long s, unsigned long e,
XEN_GUEST_HANDLE_PARAM(e820entry_t) buffer_param;
XEN_GUEST_HANDLE(e820entry_t) buffer;
- if ( ctxt->n + 1 >= ctxt->map.nr_entries )
- return -E2BIG;
- ent.addr = (uint64_t)ctxt->s << PAGE_SHIFT;
- ent.size = (uint64_t)(s - ctxt->s) << PAGE_SHIFT;
- ent.type = E820_RESERVED;
- buffer_param = guest_handle_cast(ctxt->map.buffer, e820entry_t);
- buffer = guest_handle_from_param(buffer_param, e820entry_t);
- if ( __copy_to_guest_offset(buffer, ctxt->n, &ent, 1) )
- return -EFAULT;
+ if ( !guest_handle_is_null(ctxt->map.buffer) )
+ {
+ if ( ctxt->n + 1 >= ctxt->map.nr_entries )
+ return -E2BIG;
+ ent.addr = (uint64_t)ctxt->s << PAGE_SHIFT;
+ ent.size = (uint64_t)(s - ctxt->s) << PAGE_SHIFT;
+ ent.type = E820_RESERVED;
+ buffer_param = guest_handle_cast(ctxt->map.buffer, e820entry_t);
+ buffer = guest_handle_from_param(buffer_param, e820entry_t);
+ if ( __copy_to_guest_offset(buffer, ctxt->n, &ent, 1) )
+ return -EFAULT;
+ }
ctxt->n++;
}
ctxt->s = e + 1;
@@ -4978,6 +4981,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
XEN_GUEST_HANDLE(e820entry_t) buffer;
XEN_GUEST_HANDLE_PARAM(e820entry_t) buffer_param;
unsigned int i;
+ bool store;
rc = xsm_machine_memory_map(XSM_PRIV);
if ( rc )
@@ -4986,9 +4990,10 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( copy_from_guest(&ctxt.map, arg, 1) )
return -EFAULT;
+ store = !guest_handle_is_null(ctxt.map.buffer);
buffer_param = guest_handle_cast(ctxt.map.buffer, e820entry_t);
buffer = guest_handle_from_param(buffer_param, e820entry_t);
- if ( !guest_handle_okay(buffer, ctxt.map.nr_entries) )
+ if ( store && !guest_handle_okay(buffer, ctxt.map.nr_entries) )
return -EFAULT;
for ( i = 0, ctxt.n = 0, ctxt.s = 0; i < e820.nr_map; ++i, ++ctxt.n )
@@ -5005,13 +5010,16 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( rc )
break;
}
- if ( ctxt.map.nr_entries <= ctxt.n + 1 )
+ if ( store )
{
- rc = -E2BIG;
- break;
+ if ( ctxt.map.nr_entries <= ctxt.n + 1 )
+ {
+ rc = -E2BIG;
+ break;
+ }
+ if ( __copy_to_guest_offset(buffer, ctxt.n, e820.map + i, 1) )
+ return -EFAULT;
}
- if ( __copy_to_guest_offset(buffer, ctxt.n, e820.map + i, 1) )
- return -EFAULT;
ctxt.s = PFN_UP(e820.map[i].addr + e820.map[i].size);
}
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 20df769..2a61e11 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -341,6 +341,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_memory_map_t);
* XENMEM_memory_map.
* In case of a buffer not capable to hold all entries of the physical
* memory map -E2BIG is returned and the buffer is filled completely.
+ * Specifying buffer as NULL will return the number of entries required
+ * to store the complete memory map.
* arg == addr of xen_memory_map_t.
*/
#define XENMEM_machine_memory_map 10
--
2.10.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 0/2] xen: modify dom0 interface for obtaining memory map
2016-12-05 16:34 [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Juergen Gross
2016-12-05 16:34 ` [PATCH 1/2] xen/x86: return partial memory map in case of not enough space Juergen Gross
2016-12-05 16:34 ` [PATCH 2/2] xen/x86: add a way to obtain the needed number of memory map entries Juergen Gross
@ 2016-12-05 16:39 ` Andrew Cooper
2016-12-05 16:43 ` Juergen Gross
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2016-12-05 16:39 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, ian.jackson, tim, jbeulich
On 05/12/16 16:34, Juergen Gross wrote:
> Today's interface to get the machine memory map in dom0 requires to
> know in advance how large the final map will be. There is however no
> way to either get only a part of the memory map or to ask the
> hypervisor about its size.
>
> This patch set enhances the XENMEM_machine_memory_map hypercall to
> solve both issues by returning only a partial memory map in case the
> supplied buffer was too small and to return the needed number of
> entries if no buffer is being supplied.
These changes appear to be a good improvement in behaviour.
However, there is a way to know the exact size of the memory map. Use
XENMEM_maximum_ram_page to find the maximum mfn, and use that to
calculate the size of the mapping.
See tools/libxc/xc_sr_common_x86_pv.c: x86_pv_map_m2p()
(Not that I think this interface is very nice either...)
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] xen: modify dom0 interface for obtaining memory map
2016-12-05 16:39 ` [PATCH 0/2] xen: modify dom0 interface for obtaining memory map Andrew Cooper
@ 2016-12-05 16:43 ` Juergen Gross
2016-12-05 17:06 ` Andrew Cooper
0 siblings, 1 reply; 13+ messages in thread
From: Juergen Gross @ 2016-12-05 16:43 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, ian.jackson, tim, jbeulich
On 05/12/16 17:39, Andrew Cooper wrote:
> On 05/12/16 16:34, Juergen Gross wrote:
>> Today's interface to get the machine memory map in dom0 requires to
>> know in advance how large the final map will be. There is however no
>> way to either get only a part of the memory map or to ask the
>> hypervisor about its size.
>>
>> This patch set enhances the XENMEM_machine_memory_map hypercall to
>> solve both issues by returning only a partial memory map in case the
>> supplied buffer was too small and to return the needed number of
>> entries if no buffer is being supplied.
>
> These changes appear to be a good improvement in behaviour.
>
> However, there is a way to know the exact size of the memory map. Use
> XENMEM_maximum_ram_page to find the maximum mfn, and use that to
> calculate the size of the mapping.
>
> See tools/libxc/xc_sr_common_x86_pv.c: x86_pv_map_m2p()
How does this help for the size of the E820 map of the physical machine
"enhanced" by the hypervisor to reflect holes for IOAPICs and IOMMU?
The problem is related to the *machine* memory map!
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] xen: modify dom0 interface for obtaining memory map
2016-12-05 16:43 ` Juergen Gross
@ 2016-12-05 17:06 ` Andrew Cooper
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2016-12-05 17:06 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, ian.jackson, tim, jbeulich
On 05/12/16 16:43, Juergen Gross wrote:
> On 05/12/16 17:39, Andrew Cooper wrote:
>> On 05/12/16 16:34, Juergen Gross wrote:
>>> Today's interface to get the machine memory map in dom0 requires to
>>> know in advance how large the final map will be. There is however no
>>> way to either get only a part of the memory map or to ask the
>>> hypervisor about its size.
>>>
>>> This patch set enhances the XENMEM_machine_memory_map hypercall to
>>> solve both issues by returning only a partial memory map in case the
>>> supplied buffer was too small and to return the needed number of
>>> entries if no buffer is being supplied.
>> These changes appear to be a good improvement in behaviour.
>>
>> However, there is a way to know the exact size of the memory map. Use
>> XENMEM_maximum_ram_page to find the maximum mfn, and use that to
>> calculate the size of the mapping.
>>
>> See tools/libxc/xc_sr_common_x86_pv.c: x86_pv_map_m2p()
> How does this help for the size of the E820 map of the physical machine
> "enhanced" by the hypervisor to reflect holes for IOAPICs and IOMMU?
>
> The problem is related to the *machine* memory map!
I clearly haven't had enough coffee yet. I was confusing my various
machine memory maps. Sorry for the noise.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread